-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Add flag set-env-var
to MixEnvironment
, and rename old ones
#11494
base: master
Are you sure you want to change the base?
Add flag set-env-var
to MixEnvironment
, and rename old ones
#11494
Conversation
c7fae12
to
379c043
Compare
set-var
to MixEnvironment, and rename old onesset-var
to MixEnvironment
, and rename old ones
379c043
to
98e69fd
Compare
98e69fd
to
0d52f4b
Compare
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.
I like that this makes the flag names more consistent, but I'm concerned about the user impact, since many users have opted in to the nix-command
feature flag. Perhaps the old names could be kept?
Another issue is that the effect of --set-env-var FOO
followed by --unset-env-var FOO
may be unexpected. These should be either mutually inclusive, or handled in the order they occur, e.g. by using a map<string, optional<string>>
to represent both setting and unsetting/keeping in a single map value. Using nullopt
for both unsetting and keeping might be a little weird, so you could also do map<string, variant<bool, string>>
instead.
This definitely needs new tests cases to
- test the new flag
- test the interactions.
0d52f4b
to
c05fe9b
Compare
I have another question following yours. $ nix run --set-env-var FOO '' --unset-env-var FOO --set-env-var FOO '' ... Since we're talking about storing it all in a |
c05fe9b
to
289f2ea
Compare
288552b
to
29ce075
Compare
I gave the tests a go, let me know what you think. |
29ce075
to
a20894e
Compare
Apparently, also fixes one more shellcheck case from #10795. |
set-var
to MixEnvironment
, and rename old onesset-env-var
to MixEnvironment
, and rename old ones
3498f11
to
153f5a5
Compare
Motivation
Closes: #11495
Closes: #10338
Context
#11490 (comment)
Ref: #11490
Priorities and Process
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.