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

Marshal() and env #41

Closed
cgt opened this issue Jan 28, 2015 · 3 comments
Closed

Marshal() and env #41

cgt opened this issue Jan 28, 2015 · 3 comments

Comments

@cgt
Copy link

cgt commented Jan 28, 2015

Why doesn't the Marshal func use environment variables?

@biztos
Copy link

biztos commented Jan 28, 2015

It also doesn't use flags, apparently. This seems like a bug.

Is there any reason for Marshal to ignore the env/flag overrides that affect GetString and friends?

@kzvezdarov
Copy link
Contributor

Disclaimer: I spent ~10 minutes reading the code, might be wrong.

Seems like the Marshal function is missing the stages that marshal the env. variables and the flags. Furthermore, the order of precedence seems to be wrong: the function sets it as defaults<config<overrides<k/v store, while the documentation states that it should be defaults<k/v store<config<env<flags<overrides.

It looks like it would be enough just to add those stages and reorder the marshaling. env would need to use WeakMarshal since the values are stored as strings, while pflags will have to have their values extracted to a map[string]interface{}. I can probably get it done in an hour or so, given that there are no special considerations for these config stores (env and pflags)?

@kzvezdarov
Copy link
Contributor

#44 fixes this, closing as fixed.

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

No branches or pull requests

3 participants