nixos: remove with lib from meta, trivial cases#371874
Conversation
Signed-off-by: lucasew <lucas59356@gmail.com>
emilylange
left a comment
There was a problem hiding this comment.
NACK.
You are breaking modules and tests, left and right, some are channel blockers, others aren't.
I assume what you do is well-intended, but please pause for a minute.
The entirety of nixpkgs is more complex than something you can simply throw regex search and string replace against.
Multiple folks tried this again and again and almost always caused a fallout.
You can't just replace some parts, be it with lib or whatever, and then check whether nil is able to parse it.
That's just one small piece of a puzzle you may or may not see fully yet.
And while I have you here, please reconsider how you approach both pull-requests as author and as reviewer.
Contributing isn't about the numbers.
And reviewing is about more than just nitpicks.
And you do not need to spam nixpkgs-review under every PR.
We've had folks do this before and that did not end well.
Also note that there is, to my knowledge, no consensus for removing with lib in tiny scopes like meta.
So I should separate this PR into more PRs, right?
My approach is a little more careful than this. I only use text replacement for well known patterns that are there everywhere. I begin using nil because it's the fastest, I could jump to eval tests directly. BTW the eval tests didn't detect issues that nil detected such as in the changelog field of some packages. As I am separating the big chungus PR into mergeable parts it's much easier to spot weird stuff. I am just automating the low hanging fruits to then bring to mergeable PRs. I don't intend to make the big chungus PR ready to merge ever, it's only a source of a initial diff for the reviewable PRs.
I setup it to run overnight and queued a few dozen of PRs. Not the best approach but I saw it unblocking a lot of people.
I should open a RFC for it. The problem with |
|
But yeah. I am being a little too strict for these details at least and I should let go and maybe fix in a follow up. |
|
|
Superseded by #443046. |
Part of #371862
Signed-off-by: lucasew lucas59356@gmail.com
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-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.