smc-fuzzer: init at 0-unstable-2020-12-23 #332586
Conversation
hraban
force-pushed
the
smc-fuzzer
branch
2 times, most recently
from
c1651c2 to
ec3a609
Compare
ofborg
bot
added
8.has: package (new)
pkgs/top-level/all-packages.nix
Outdated
There was a problem hiding this comment.
Is this change necessary? Seems a bit off-topic for a package addition.
There was a problem hiding this comment.
totally unnecessary I just came across it and thought I'd clean up while I saw it. I created a separate commit for it but I can also create a new PR if that's better.
I used to be stricter about creating separate PRs for every little change but I kinda got told off by other nixpkgs maintainers for it so I'm a bit more lenient now.
There was a problem hiding this comment.
I'm not a committer, so I don't know how they feel about it, but it doesn't make sense to me to bundle completely unrelated changes in one PR. Separating changes makes it easier to review and merge them too, as a minor alphabetical reordering like this one wouldn't need to wait for the main package commit to pass reviews.
There was a problem hiding this comment.
I'm gonna stick with the feedback I got from the nixpkgs maintainer at the time which was the opposite of this, but if a maintainer comes in and dictates otherwise I'll change my mind.
Tbh I kind of agree with them: it's easier, I'm an unpaid volunteer so fewer PRs = less work, it's also less work to review, there is zero rush for this to get merged so the actual value of "not having to wait" is zero. 🤷 If I'm being honest with myself: at this point if I had to create a separate PR for every little change I'd just stop contributing little changes.
(s/maintainer/committer/ but you get the idea)
There was a problem hiding this comment.
That's fair, and I'd approve either solution anyway. We should really have a CI check for a sorted all-packages.nix (or better yet: no all-packages.nix at all!) so contributors don't need to worry about it.
There was a problem hiding this comment.
I will not complain about such a minor, cosmetic thing.
However I should say it is mostly unneeded because:
-
The all-packages file will never be in alphabetic order, since we have no way of enforcing order in such a huge file given the truckload of merges and rebases happening in parallel
-
RFC 140 will make this file obsolete - or at least legacy.
- Especially the enforcement for future packages: [RFC 140 2a]
pkgs/by-name: Enforce for new packages #275539 - Especially the mass migration: [RFC 140 2b]
pkgs/by-name: Automated migration #211832
- Especially the enforcement for future packages: [RFC 140 2a]
There was a problem hiding this comment.
Fair point--removed. Thank you.
There was a problem hiding this comment.
That's fair, and I'd approve either solution anyway. We should really have a CI check for a sorted
all-packages.nix(or better yet: noall-packages.nixat all!) so contributors don't need to worry about it.
This is also a good point you're right
hraban
force-pushed
the
smc-fuzzer
branch
2 times, most recently
from
965826b to
ce6e055
Compare
|
Thanks for your reviews @AndersonTorres and @SigmaSquadron 👍 |
hraban
changed the title
SigmaSquadron
added
the
12.approvals: 2
wegank
removed
the
12.approvals: 2
Description of changes
smc-fuzzer "https://github.com/theopolis/smc-fuzzer/" is originally written to fuzz the SMC chip on apple laptops, but it's actually quite useful if you just want to have bona fide interaction with the SMC.
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.