| 26 Jul 2025 |
emily | like within next couple days l | 09:31:28 |
emily | * | 09:31:34 |
emily | there's a big security fix | 09:31:38 |
emily | so if you can wait a week I'd wait a week | 09:31:44 |
niklaskorz | I see, let’s add that in the treewide PR so it’s documented | 09:32:12 |
emily | unless it's an emergency. did someone open a tracking issue to remove them all by hand? 😆 | 09:32:13 |
niklaskorz | will add a comment | 09:32:17 |
emily | the first time I saw someone nitpicking about a new package set the flag was when I realized we gotta make it warn | 09:32:56 |
emily | * | 09:33:09 |
emily | I mean FWIW I'm not strongly against merging into master now | 09:33:45 |
emily | if people are super impatient about doing it manually | 09:33:55 |
niklaskorz | hm yeah, if staging lands in a week that also means we’d only have to deal with backport conflicts for a week | 09:34:53 |
emily | this is a collective action problem that could be solved by everyone choosing to just be chill about one useless variable in their packages :P | 09:35:39 |
emily | but I remember how it was with the formatter so | 09:35:45 |
niklaskorz | if us nixpkgs contributors can do one thing well, it’s not acting coordinated! | 09:36:40 |
emily | In reply to @niklaskorz:matrix.org I’d suggest anyone reading here to stop requesting PRs to remove it themselves, except for new packages of course FWIW I encourage you to push back very hard on this | 09:41:54 |
emily | since this request is definitionally out of scope for any PR that isn't intended as a general cleanup and that doesn't add a new package | 09:42:25 |
emily | I wish we would more proactively reject such requests as unreasonable and hide the comments | 09:42:50 |
niklaskorz | I’m fine with either honestly, and have the according comment written and ready to submit on the treewide. So ultimately I’m leaving it to @Toma now whether I should press merge or press „submit review comment". | 09:44:45 |
emily | my personal feeling is that teaching a week of more new reviewers that requesting unrelated sed scripts be run isn't how we ought to do things is higher value than getting it merged now, but I've only personally seen one or two nitpicks on the matter so far so I might be out of touch :) | 09:46:35 |
emily | I will defer the judgement to others | 09:46:41 |
emily | however | 09:46:43 |
emily | I do believe we want to ship a warning on master ASAP after a treewide | 09:46:58 |
emily | because otherwise new ones will slip in or worse we'll still get nitpicks. if CI fails because of the warning a machine can handle it | 09:47:29 |
Toma | Absolutely, I just wanted the PR to be reproducable :) | 09:47:46 |
niklaskorz | yeah really good job on that, thanks 😄 | 09:48:18 |
emily | yeah I just mean, waiting until we have a PR for the warning seems good. (but I assume that would only take a couple minutes so it doesn't speak to waiting for the cycle or not) | 09:49:05 |
Toma | I will be away for a week, so I wont be able to handle this myself
But I'd say the warning PR should be like
`useFetchCargoVendor ? null` and `warnIf (useFetchCargoVendor != null) ......`
About whether we merge the main PR now: uuhhh, I'm leaning towards not wating for stable, but IDK
| 09:52:53 |
niklaskorz | just for clarification: useFetchCargoVendor = false; is not and will not be a thing? | 09:54:12 |
emily | hasn't been since 25.05 | 09:54:28 |