Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace "." characters with "_" when parsing environment variables #163

Closed
wants to merge 3 commits into from

Conversation

four2five
Copy link

This change allows the parsing of environment variables that correspond to nested keys. This seems like a safe conversion since "." is not allowed in environment variables.

Joe Buck added 3 commits February 18, 2016 21:24
The field is being added in preparation for testing nested key parsing
from environment variables.
Add a test where an environment variable with a nested key
is set and then retrieved. The nested key uses an underscore
in place of a dot to indicate the key hierarchy.
This brings viper in line with other configuration parsing libraries
and in line with what I would anticipate are the expectations of most
programms.
@CLAassistant
Copy link

CLAassistant commented Feb 19, 2016

CLA assistant check
All committers have signed the CLA.

@four2five
Copy link
Author

I'm waiting on clearance from my work to sign the CLA. Should have it today or tomorrow. Would love some comments on the patch in the interim.

@chancez
Copy link
Collaborator

chancez commented Apr 2, 2016

Why not just use SetEnvReplacer(string...)?

@four2five
Copy link
Author

Yeah, that seems to be the route forward. I need to rework this when I have a bit more time. Thanks for the suggestion.

@four2five
Copy link
Author

I intend to rework this patch to remove my code change and to update the test cases to use SetEnvReplacer , as suggested. That would (IMHO) add value as an example of how to solve this issue and as a valuable test case.

@awfm9
Copy link

awfm9 commented Sep 21, 2016

If you still want to write test cases for the SetEnvReplacer, feel free to create a new pull request. I will close this PR for the time being.

@awfm9 awfm9 closed this Sep 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants