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

Don't post mentions for draft PRs. #1638

Merged
merged 1 commit into from
Jul 18, 2022

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Jul 17, 2022

The mentions can be quite noisy, so this provides a way to prevent them from being posted. Presumably a draft PR is under construction and may change over time, so the mentions may be premature. Once the author clicks "Ready for review", triagebot will re-scan the PR and post any mentions that haven't already been posted.

cc #1637

@Mark-Simulacrum
Copy link
Member

We should try to see if we can ping the moment a PR exits draft status, otherwise it can move to being merged without being pinged very easily.

(Not sure what hook to listen on for that, if any).

We probably also ought to make bors error on r+ for draft PRs, but that's less important.

@ehuss
Copy link
Contributor Author

ehuss commented Jul 18, 2022

We should try to see if we can ping the moment a PR exits draft status, otherwise it can move to being merged without being pinged very easily.

Perhaps I'm misunderstanding the question, but this should do that. When the user clicks "Ready for Review", the webhook sends a ReadyForReview action, which should then immediately post any mentions. here is an example where immediately after the "marked this pull request as ready" event, it posts the mention comment.

@Mark-Simulacrum
Copy link
Member

Oh, yes, indeed. Sorry, must have read the diff too quickly.

This looks good to me then, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants