Skip to content
This repository has been archived by the owner on Aug 14, 2020. It is now read-only.

[issue/690] allow . and - in environment variable names. #691

Merged
merged 1 commit into from
Aug 7, 2017

Conversation

ae6rt
Copy link
Contributor

@ae6rt ae6rt commented Jul 16, 2017

Files touched: schema/types/environment.go, schema/types/environment_test.go

Allow additional characters in the set considered valid in environment variable names. Prompted by
appc/docker2aci#244 and rkt/rkt#3532.

@ae6rt
Copy link
Contributor Author

ae6rt commented Jul 16, 2017

@lucab PTAL, taking you up on your offer: rkt/rkt#3532 (comment)

@ae6rt
Copy link
Contributor Author

ae6rt commented Jul 16, 2017

The IEEE spec from which the original conservative character set is taken, as a reference:

Editing with a more precise link:

http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html

@lucab
Copy link
Contributor

lucab commented Jul 17, 2017

@ae6rt thanks for the PR, can you please also update SPEC.md for the new characters?

Files touched:  schema/types/environment.go, schema/types/environment_test.go, spec/aci.md

Allow additional characters in the set considered valid in environment variable names.  Prompted by
appc/docker2aci#244 and rkt/rkt#3532.
@ae6rt
Copy link
Contributor Author

ae6rt commented Jul 17, 2017

I updated spec/aci.md, and rebased. I wanted to honor the desire to adhere to the IEEE spec, so I left the word 'solely' in, with a qualifying clause about the practical desire to allow . and -. I hope it doesn't read strangely.

Done. PTAL @lucab

@ae6rt
Copy link
Contributor Author

ae6rt commented Aug 3, 2017

Ping @lucab knowing you are busy, but I don't want to forget about this.

@lucab
Copy link
Contributor

lucab commented Aug 4, 2017

LGTM, but I don't have merge powers here.

@cgonyeo cgonyeo merged commit 210e299 into appc:master Aug 7, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants