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

Breaking change. New parsing .env files logic #127

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

wawan93
Copy link

@wawan93 wawan93 commented Jul 31, 2023

What changed

  1. ReadConfig only parses files, and doesn't load env variables.
  2. ReadConfig with .env file doesn't change os envs.
  3. ReadEnv updates fields, read earlier from ReadConfig

Why

As stated in Contributing.md,

... this project is designed to be minimalistic, obvious, and as simple as possible. We prioritize clear and easy-to-understand code over complex or "magical" solutions.

But, right now the behavior of ReadConfig is not obvious at all. And, in case with .env - it's "magical". When I see two methods ReadEnv and ReadConfig, I expect that ReadEnv reads config only from envs, not from files; and ReadConfig only reads from a file, not from env variables. And If I need to do both, I should call both methods. But if I call ReadConfig with .env file, I can't read env variables anymore, because they are changed.

I suggest changing this behavior: make ReadConfig read only from the file, not from the env; and make it work correctly with .env files (not to change os.GetEnv behavior).

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.

None yet

1 participant