-
-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
treewide: improve prepending and appending derivation arguments in bash code #357052
treewide: improve prepending and appending derivation arguments in bash code #357052
Conversation
BTW, something that was pointed out to me is that |
I went through this PR line-by-line again, LGTM. Can't approve myself, though ;) |
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.
LGTM modulo a couple nits.
9444c3b
to
1c2a5af
Compare
No idea why Outpaths (aarch64-darwin) suddenly fails after rebasing. |
This had been resolved in NixOS#318226.
1c2a5af
to
f81eada
Compare
Just running it once more fixed it. |
@emilazy in case you are ok with this PR as well: ofborg-eval passed. |
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.
LGTM, great to see the structuredAttrs support getting more in shape. Also generally a nice cleanup of all those little accumulated hacks.
f81eada
to
801fe2c
Compare
…sh code Those would be problematic with __structuredAttrs turned on, because they'd turn those nice bash arrays back into strings - and potentially lose some of the values on the way.
801fe2c
to
d86588d
Compare
Those would be problematic with __structuredAttrs turned on, because they'd turn those nice bash arrays back into strings - and potentially lose some of the values on the way.
Part of the overall effort to enable structuredAttrs by default eventually, tracking: #205690
I think those changes are straight-forward, but somebody better double check.
@ShamrockLee @emilazy
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.