workflows/eval: add eval summary before requesting reviewers#371528
workflows/eval: add eval summary before requesting reviewers#371528infinisil merged 1 commit intoNixOS:masterfrom
Conversation
This is to ensure that the eval summary is still set as commit status, even when the review requests fail due to too many reviewers.
github-actions
bot
added
6.topic: policy discussion
nix-owners
bot
requested review from
LeSuisse,
Mic92,
azuwis,
mweinelt,
risicle and
zowoq
github-actions
bot
added
10.rebuild-darwin: 0
|
Will this prevent CI from showing up as failed when this happens? |
|
No, the job is showing as failed on purpose until we implement something better for that - or decide that silently non-failing is the right thing to do. Still working on that. |
|
I think it’s unfortunate that it makes it look like there’s an eval problem with the PR on every treewide. I would prefer the GitHub grey status that ofborg used to use for this. (It also means I get an email for every merge into |
100%, so would I. Unfortunately - not that easy to do :D |
|
Well, I’d still take green over red if those are the two choices, because of the email thing :) |
|
How about continue-on-error |
This just happened to me - and I not only got emails for Eval / Tag, but also for various other checks (nixfmt, nixpkgs-vet, one more I think). I guess some pull request jobs should just not run in staging-next, right? It's technically a PR, yes, but we are already past the PR stage. We should treat it similar to the master branch in terms of CI, I think. |
That would still make the tag job itself green in that case. If we instead split the review request itself from "preparing the list of maintainers", we could put the condition "if more than 10 reviewers" into a GHA |
|
Let's make it succeed, failing for that really isn't right: #375354 |
It's better than getting failed CI and emails: #371528 (comment) And fix a shellcheck lint
It's better than getting failed CI and emails: #371528 (comment) And fix a shellcheck lint (cherry picked from commit 06ee611)
3 participants
This is to ensure that the eval summary is still set as commit status, even when the review requests fail due to too many reviewers.
Noticed by @emilazy in #370995 (comment).
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.