workflows/eval: no maintainer reviews in draft mode #372479
Conversation
|
FWIW I always felt like this was a misfeature of the GitHub codeowners mechanism, not something to replicate. |
I'm fine with removing it for both maintainers and codeowners - for me, consistency matters the most here. I'd be surprised if only half of the requests are done in draft mode. @infinisil WDYT? |
|
As a sometimes shy PR author, I like the GitHub draft-means-no-pings behavior, since it lets me see the CI run before "presenting" the work as ready. |
Interesting. Personally, I have had more cases of "I'd like to get some feedback from others already, but make it clear that this is not merge-ready from my side, yet". This would then be "draft + reviewer requests". Edit: The missing piece here is "blocking self-review", which I would like to have done a few times. But I guess, I could pick my initial reviewers manually as well, while you can't remove all of them once they are pinged. |
That's what I do, yeah. |
We intend to use the edited event to react to base branch changes - but before this change, we also ran those jobs on simple edits like title or description. While this works for some of the quicker jobs, it will not be sustainable for all evaluation-related jobs. But evaluation needs to be re-triggered on a base branch change as well, thus this change.
When the base branch changes, we need to run full eval again, because the changed output paths depend on the base branch.
f93f6c8 to
c765b6c
Compare
|
So, summarizing. There are two opinions on the matter of whether to ping or not to ping while in draft. When not pinging, those who want reviews while in draft, can always request them, but not the other way around. Why would someone not want reviews while in draft? Because they'd like to see CI proceed. The underlying problem here is: We can't easily run CI locally to confirm that everything is green before pushing. Thus, I propose:
|
|
FWIW I think drafts are a useful signal of “still in design phase, not ready for merge yet” but that, conversely, I am often pinged on a PR that just came out of a lengthy draft stage and really wish I had known about it sooner so that I could have provided design feedback at an earlier stage. I accept that this is not fully reconcilable with shyness in putting a PR up – for my part I tend to only open PRs when I feel pretty confident about the general direction in the first place – but just explaining why I would personally prefer pings on drafts. (But I 100% agree that we need the ability to replicate our CI locally. It would be great to avoid lock‐in too.) |
|
One scenario where no-ping-draft-mode was used is «make pull request against master, run CI, oh no too many rebuilds, draft, retarget to staging, undraft». |
|
Thankfully that’s no longer relevant since the new CI pinging stuff detects that case. |
This is based on #371216 and #372475, only the last commit is relevant for this PR.
This avoids requesting maintainers for review by enabling DRY_MODE similar to codeowners. (#370857 (comment))
Not tested, yet.
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.