| 26 Jul 2025 |
 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 |
| emily | that's why the nitpicking is dumb | 09:54:32 |
| emily | we will remove the warning after a release cycle | 09:54:41 |
| niklaskorz | the warning or the attribute? | 09:54:54 |
| emily | and it will become an implicit derivation var like all other mkDerivation args | 09:54:55 |
| emily | the warning. the attribute doesn't exist (after the -next PR) | 09:55:08 |
| emily | it's just an ignored argument | 09:55:18 |
| niklaskorz | I see, perfect | 09:55:25 |
| emily | In reply to @tomasajt:matrix.org
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
maybe args ? useFetchCargoVendor is better | 09:55:42 |
| Toma | That is better | 09:56:35 |
| emily | thanks for all the work you've done lately btw. try not to check notifications too much while you're away :) | 09:56:59 |
| emily | In reply to @emilazy:matrix.org
maybe args ? useFetchCargoVendor is better we will also need to reword the throw for the false case to just tell people to remove it | 09:57:24 |
| niklaskorz | "+1“ on that! | 09:57:24 |
| emily | and then after branch-off we drop 'em both | 09:57:35 |
| emily | or rather | 09:57:40 |
| emily | we leave a TODO saying to do that and then forget about it | 09:57:49 |
| emily | (for bonus points you can prepare the follow-up immediately and label it waiting for branch-off) | 09:58:21 |
| emily | In reply to @emilazy:matrix.org
we leave a TODO saying to do that and then forget about it there's stuff in lib saying like drop me in 23.05 | 09:58:55 |
| emily | every time I see it I consider dropping it but I sort of want to see how long it can last | 09:59:24 |
| emily | like waiting to see if it'll grow mould or something | 09:59:47 |
