-
Notifications
You must be signed in to change notification settings - Fork 1
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
- Change the order in which the environment variables from the .env f… #6
base: main
Are you sure you want to change the base?
Conversation
…ile are merged with the OS environment variables. Variables from the .env file can override OS environment variables. - Add the possibility within the .env file to refer to variables within the .env file or to replace the placeholder variable with the value from the OS environment variable.
…vironment variables or if the placeholder is empty.
Interesting, thanks for the ideas. These are two distinct changes:
In the meantime, if you're ok with putting the overriding reversal behind a flag, we could just merge that first part. |
main.go
Outdated
} | ||
return part | ||
} | ||
log.Printf(`Replacing the variable failed, env key is empty: "%s"`, part) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably only in verbose mode ?
.env.demo
Outdated
@@ -7,3 +7,6 @@ DOT.TED=dotted | |||
SHARP=sharp after a var # is not a comment | |||
# This is a most likely override | |||
SHELL=invalid | |||
YOUR_NAME=Felix | |||
GREETING=Hello ${YOUR_NAME}! | |||
TEST_HOME_PATH=${HOME} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing terminal LF
Makefile
Outdated
@@ -1,2 +1,4 @@ | |||
demo: | |||
LOCAL=demo go run . -f .env.demo env | sort | |||
demo-convert: | |||
LOCAL=demo2 go run . -f .env.demo env | sort | sed -E 's/(.*)/-e \1/' | tr '\n' ' ' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing terminal LF : looks like an editor configuration issue ; maybe add a .editorconfig for that ?
@@ -138,7 +162,9 @@ func main() { | |||
}() | |||
|
|||
env := envFromReader(rc) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned in the general comment, this reverses the overriding logic, so I think it is better to put it behind a CLI flag.
https://www.npmjs.com/package/env-cmd
The Laravel framework uses exactly this format Also Docker uses the same format e.g. in the Docker files etc.
Variables makes sense in several cases
Ok, I haven't thought about default values yet, would be cool of course, so without and then if you use
Sure, I don't have a problem with that, I could just imagine that sometimes it can be really useful to overwrite certain environment variables, e.g. for the development environment. Best, Felix |
Thanks, nice project, can use it well myself right now.
But I have extended your project a bit. Maybe you can use it too. What do you say to my changes?
Change the order how the environment variables from the .env file merged with the OS environment variables.
Variables from the .env file can override OS environment variables.
Add the possibility within the .env file to refer to variables within the .env file or to replace the placeholder variable with the value from the OS environment variable.