| 27 Sep 2023 |
infinisil | I'll probably go for that regarding RFC 140 | 17:38:41 |
infinisil | In short: Run both the old and new CI checks, on both the base and merged state, then figure out when it makes sense to fail the check based on that | 17:39:33 |
@piegames:matrix.org | Why so complicated? GitHub actions already always run on the merged code (IIRC), so simply pin the version in the repo and you're done? | 17:40:54 |
infinisil | piegames: Because the CI check might be outdated | 17:41:32 |
infinisil | CI check runs successfully, master branch updates with stricter checks, PR gets merged, master is broken | 17:41:50 |
infinisil | With a strategy as described, you can kind of guarantee that any new PRs won't break master if merged | 17:43:29 |
@piegames:matrix.org | Ah. Yeah, merge bot. Or have a bot that forces a rebase after any CI changes | 17:43:46 |
infinisil | Hard to have a good definition for CI changes though. The entire repo can influence CI | 17:44:41 |
@piegames:matrix.org | In reply to @infinisil:matrix.org CI check runs successfully, master branch updates with stricter checks, PR gets merged, master is broken Also, this is not the problem desribed in your issue, as far as I read it | 17:45:11 |
infinisil | But maybe one could tag commits with "if this gets merged, the CI checks for all PR's need to be re-triggered" | 17:45:31 |
@piegames:matrix.org | In reply to @infinisil:matrix.org Hard to have a good definition for CI changes though. The entire repo can influence CI CI is a derivation and if it changes? ^^ | 17:45:38 |
infinisil | In reply to @piegames:matrix.org Also, this is not the problem desribed in your issue, as far as I read it True, it's applicable in the same way though | 17:45:57 |
infinisil | Hmm let me see if it's possible to somehow re-trigger checks | 17:47:07 |
infinisil | Though, poor ofborg could just die from that.. | 17:47:17 |
infinisil | Okay maybe we could be very specific with it. Only re-trigger CI for PR's that touch all-packages.nix | 17:47:59 |
@piegames:matrix.org | How about making stricter checks soft-fail for a while at first instead? | 17:48:52 |
infinisil | That's kind of what https://github.com/NixOS/nixpkgs/issues/256788 is about | 17:49:30 |
@piegames:matrix.org | I must admit that I am completely lost in the issue text, after reading it twice | 17:51:02 |
infinisil | Yeah sorry I admit it's really hard to read.. | 17:51:15 |
infinisil | Just a wall of text 😅 | 17:51:24 |
infinisil | For the nixcon presentation it was fun trying to come up with visual explanations of things, I'll try the same here :) | 18:08:28 |
infinisil | piegames: https://github.com/NixOS/nixpkgs/issues/256788#issuecomment-1737965650 | 19:33:48 |
@piegames:matrix.org | https://github.com/NixOS/nixpkgs/issues/256788#issuecomment-1737979351 yeah I think this was the actual cause for my confusion | 19:48:32 |
infinisil | Yeah, well I'm glad I was able to explain it, it's useful specific to nixpkgs-check-by-name, but a CI retrigger is a decent fix the other general problem :) | 19:49:34 |
infinisil | * Yeah, well I'm glad I was able to explain it, it's useful specific to nixpkgs-check-by-name, but a CI retrigger is a decent fix for the other general problem :) | 19:49:44 |
infinisil | Oh and I guess this could also be useful if it's too expensive to re-trigger the CI check for all affected PR's | 19:52:08 |
infinisil | (not the case for nixpkgs-check-by-name) | 19:52:22 |
infinisil | Kind of nice how this approach allows a gradual rollout of a new CI check though. And you can kind of control the amount of the rollout based on how quickly you fix the base branch when the new check starts failing on it. | 19:58:30 |
infinisil | * Kind of nice how this approach allows a gradual rollout of a new CI check without using any extra resources though. And you can kind of control the amount of the rollout based on how quickly you fix the base branch when the new check starts failing on it. | 19:58:46 |
infinisil | Nice, the RFC 140 tool prevented the double addition of a package: https://github.com/NixOS/nixpkgs/pull/257565 | 21:12:28 |