| 27 Sep 2023 |
@piegames:matrix.org | And is it worth the effort? | 17:33:44 |
infinisil | I guess merge trains would improve on that | 17:33:45 |
K900 | In reply to@piegames:matrix.org And is it worth the effort? I think it's worth considering in the general effort to fix our CI situation | 17:34:14 |
K900 | Is it worth doing by itself? Probably not | 17:34:24 |
K900 | Is it worth doing as part of a bigger project to replace hydra/ofborg/etc? Absolutely yes | 17:34:49 |
infinisil | piegames: On the scale of Nixpkgs it might be, because we have so many PR's, many of which linger around for a while | 17:34:53 |
infinisil | Any CI checks that newly fail would then break on master when those PR's are merged | 17:35:11 |
infinisil | In particular I'm thinking of https://github.com/NixOS/nixpkgs/pull/211832#issuecomment-1732153092 here | 17:35:54 |
@piegames:matrix.org | Btw this has a hard dependency of going through a merge bot for everything | 17:36:14 |
infinisil | The first step should be to disallow definitions in all-packages.nix when they should be in pkgs/by-name instead. But if we just put that in a CI check, it would fail regularly for probably some days as people merge all-packages.nix changes in | 17:36:48 |
K900 | In reply to@piegames:matrix.org Btw this has a hard dependency of going through a merge bot for everything Not necessarily | 17:36:57 |
K900 | But I also don't think this is too bad, generally | 17:37:13 |
@piegames:matrix.org | Oh I absolutely do want a merge bot, and soon pretty please | 17:37:49 |
infinisil | The idea given here should help for some cases: https://github.com/NixOS/nixpkgs/issues/256788 | 17:38:34 |
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 |