matthiaskrgr
commented
2 years ago Triagebot could ignore the pings as long as the ghost user is assigned and do the ping only once someone requests an actual reviewer via r? maybe?
Open RalfJung opened 2 years ago
matthiaskrgr
commented
2 years ago Triagebot could ignore the pings as long as the ghost user is assigned and do the ping only once someone requests an actual reviewer via r? maybe?
RalfJung
commented
2 years ago I guess that would somewhat replicate the old "accidentally silent" system that highfive had.
However, for Miri PRs, I do like the rustbot pings so I don't have to do them. I guess it could do the pings both on r? and when it sees a bors message saying "commit blah has been approved" (but not in rollups)?
Mark-Simulacrum
commented
2 years ago I think using GitHub's draft PR status might be a good fit here. It won't prevent any interaction with bots (including bors, afaik) and seems like a reasonable "should be silent as an experiment that won't land" status. Also easy to notice and know to clear as an author, hopefully.
I think we may want a separate mechanism too (similar to special case for rollups), that's just indicative that this PR shouldn't be notified on - but I'm not sure how large the remaining territory is where pings are annoying and draft PR doesn't make sense.
ehuss
commented
2 years ago Turning off mentions for draft PRs sounds like a good idea to me. I posted #1638 to implement that.
Unfortunately I suspect most people won't know to use draft PRs for things like performance testing, so I'm not sure it will be particularly effective. I also like the idea of not pinging on unassigned/ghost PRs, but it sounds like that won't work for Ralf's case. I also suspect that any "don't mention" tagging mechanism won't be particularly effective since just like draft PRs, I don't think people will know to do that.
I'm not sure I have any ideas other than looking at the assignee to avoid it.
RalfJung
commented
2 years ago Is it even possible to mark a PR as draft during creation (so that it applies for the initial pings)?
jyn514
commented
2 years ago Is it even possible to mark a PR as draft during creation (so that it applies for the initial pings)?
Yes, it shows up on the "Create Pull Request" button (there's a little drop-down that lets you pick "Create Draft Pull Request" instead).
RalfJung
commented
2 years ago Yes, it shows up on the "Create Pull Request" button (there's a little drop-down that lets you pick "Create Draft Pull Request" instead).
Wow that is super well-hidden, even Ctrl-F "draft" does not find it. You have to basically know about it to be able to find it...
jyn514
commented
2 years ago yeah, lots of github's ui is super unintuitive ... took me like a year to figure out that y gets you a permalink to the current line in the source
PRs like https://github.com/rust-lang/rust/pull/99368 cause a lot of premature pings -- it's just a perf experiment, we don't have to all know about that.
So maybe it should be possible to somehow tell triagebot to not do mentions for a particular PR? We have a special exception for rollups that does this I think, but other PRs might also want it. Ideally, if it turns out the perf experiment gives the expected results and we do want to land that PR, there is a way to tell triagebot to now please do the pings that have been suppressed so far.