| 27 Sep 2023 |
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 |
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 |