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

Bot should ignore draft pull requests #57

Open
sffc opened this issue Feb 3, 2023 · 6 comments
Open

Bot should ignore draft pull requests #57

sffc opened this issue Feb 3, 2023 · 6 comments
Labels
enhancement New feature or request

Comments

@sffc
Copy link
Member

sffc commented Feb 3, 2023

When a PR is in early development, the bot can be noisy. Given that the bot's primary purpose is to help with code reviews, there should be an option (or perhaps make it the default behavior) to suppress notifications if a PR is in Draft state.

CC @Manishearth

@sffc sffc added the enhancement New feature or request label Feb 3, 2023
@srl295
Copy link
Member

srl295 commented Feb 8, 2023

@sffc yes, however a PR could move from draft to final before the bot gets a chance to weigh in. So I might propose that the bot still update the status of the PR, just be a little less noisy.

Also as far as noise, a bot can update prior messages instead of just appending new messages. That removes the history, but does reduce visual noise (not email noise).

@Manishearth
Copy link
Member

I find the history useful, fwiw.

however a PR could move from draft to final before the bot gets a chance to weigh in

What's the problem with that? All of the pushes will have happened in draft mode, no?

@sffc
Copy link
Member Author

sffc commented Feb 9, 2023

My guess, having not researched the answer, is that the webhook probably is already running when a PR changes from Draft to Open, because the webhook is triggered on pr:changed or something like that.

@srl295
Copy link
Member

srl295 commented Feb 9, 2023

I find the history useful, fwiw.

Even the history while in draft mode?

however a PR could move from draft to final before the bot gets a chance to weigh in

What's the problem with that? All of the pushes will have happened in draft mode, no?

The potential problem would be if a user is allowed to merge a PR that didn't get checked by the PR checker, because it was ignored while in draft mode.

@Manishearth
Copy link
Member

Even the history while in draft mode?

Oh, no, not in draft mode

The potential problem would be if a user is allowed to merge a PR that didn't get checked by the PR checker, because it was ignored while in draft mode.

Yeah in that case having the main PR check comment get edited seems fine, it just shouldn't complain about force pushes.

@Manishearth
Copy link
Member

How about this as a concrete proposal: Bot ignores force pushes on draft PRs.

For the other stuff we can have it edit if we want but it's not something ICU4X needs particularly.

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

No branches or pull requests

3 participants