Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for "silent" PRs? #1637

Open
RalfJung opened this issue Jul 17, 2022 · 8 comments
Open

Support for "silent" PRs? #1637

RalfJung opened this issue Jul 17, 2022 · 8 comments
Labels
A-mentions Area: PR auto mentions

Comments

@RalfJung
Copy link
Member

PRs like rust-lang/rust#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.

@matthiaskrgr
Copy link
Member

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
Copy link
Member Author

RalfJung commented Jul 17, 2022

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
Copy link
Member

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
Copy link
Contributor

ehuss commented Jul 17, 2022

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
Copy link
Member Author

Is it even possible to mark a PR as draft during creation (so that it applies for the initial pings)?

@jyn514
Copy link
Member

jyn514 commented Jul 17, 2022

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
Copy link
Member Author

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
Copy link
Member

jyn514 commented Jul 22, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-mentions Area: PR auto mentions
Projects
None yet
Development

No branches or pull requests

5 participants