-
-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
unixtools: refactor for less redundancy #40134
Conversation
b1bad48
to
898e4b0
Compare
pkgs/top-level/unix-tools.nix
Outdated
}; | ||
logger = { | ||
linux = pkgs.utillinux; | ||
darwin = more_compat; |
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.
This shouldn't be included.
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.
Yeah, right you are, it was an accidental paste and I mistook it for an accidental deletion
898e4b0
to
4b93f21
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.
Looks good! Not huge line saving but will make it easier to add more.
pkgs/top-level/unix-tools.nix
Outdated
darwin = pkgs.darwin.basic_cmds; | ||
|
||
makeCompat = name': value: buildEnv { | ||
name = name' + "-compat"; |
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.
Make sure these have 2 space indentation.
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.
✔️
(Looks like master machine broke, probably shouldn't merge this until that is fixed, just to make sure it won't trigger a mass rebuild) |
The eval error was hopefully unrelated. @GrahamcOfBorg eval |
Looks like its fine now |
Thanks! |
Unfortunately this complicates merging with staging, which made changes to unix-tools before the reformatting. Can someone take a look? Thanks! |
Just merged master into staging. |
@matthewbauer haha thanks! 😸 |
Wew, that was quick 😅 |
Motivation for this change
unix-tools
didn't quite follow the DRY principle, refactored it to follow this principle better.This shouldn't actually change the output of any derivation, just the way we get there, so @GrahamcOfBorg shouldn't give any rebuilds.
Things done
build-use-sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)