search

pullreminders / backlog

Public backlog for http://pullpanda.com
59 stars 9 forks source link

Feature Request: Ignore bot-created PRs in pull-assigner #142

Open 5outh opened 5 years ago

5outh commented 5 years ago

My organization uses restyled.io to draft PRs that automatically restyle our code. When restyled creates a PR, it touches the code paths that trigger pull-assigner to automatically assign a reviewer to it. pull-assigner has been awesome for us, but this one litle bit of noise is unhandled.

Can we introduce a way to ignore bot-created (or particular-user-created) PRs with pull-assigner?

abinoda commented 5 years ago

@5outh Yes – we can add this shortly.

abinoda commented 5 years ago

@5outh Can you try this first: configure restyled to add a pullassigner-ignore label to its pull requests? Pull Assigner will then ignore these PRs. See here: https://github.com/restyled-io/restyled.io/issues/121

5outh commented 5 years ago

@abinoda We did this; the pullassigner-ignore label works when creating a PR in the UI and adding the label before submitting the PR, but when we hooked it up with restyled, the label must be added after PR submission, so we see this:

fafa

Can a delay be added to account for this type of blip, or something to that effect?

pbrisbin commented 5 years ago

:wave: hi there. I think I actually know what's going on. And (if I'm right) it's a slightly more generic issue than just this ignore-by-label behavior.

I suspect you all get a PullRequest object in the webhook payloads you receive, and I'll bet you work directly with this resource instead of asking GitHub for the PR again via their API at any later point -- why would you?

Doing it that way actually causes surprising behaviors on any programmatically-created PRs: while the GitHub web UI will let you create a PR with labels or reviewers in one action, the API will only let you create a PR, then add labels or pick a reviewer for that just-created PR. This means that the PRs created by Restyled won't have labels on them when you get them in your webhooks, even though those labels were added "instantly" when the PR was created. Similarly, when I create a PR with hub pull-request -r <some reviewer>, pull-assigner doesn't see the reviewers that were requested "instantly" either, so it still does its thing:

(Interestingly, this bug is actually a feature in the eyes of #149.)

I'm not sure what to do for this. It would be kind of crappy to change your processes to re-query the GitHub API for PR details later, when you already have it in the webhook payload, but I also think this kind of automation that causes labels, reviewers, or other details to be added to the PR separately from its creation to only get more popular. And I'm not sure how to keep apps like pull-assigner working, other than to re-query the PR details again as late in the process as possible.

abinoda commented 5 years ago

@5outh @pbrisbin Interesting, and thanks for the additional thoughts! Just confirm, the problem here is that the CODEOWNER review requests are triggered before the labels are added.

I think the best solution for now may be to allow "ignoring" specific users, as @5outh originally suggested. I don't think this should be too difficult... will scope it out over the weekend.

pbrisbin commented 5 years ago

Just confirm, the problem here is that the CODEOWNER review requests triggered are before the labels are added.

That's correct. Though I might clarify that CODEOWNER review requests will always be triggered before the labels are added in the case of programmatically-created PRs (since the GitHub API only supports create-then-label as two distinct actions). The problem is that pull-assigner is reacting to the PR as it was when created, and is not seeing the labels that were added instantly-but-separately.

I think the best solution for now may be to allow "ignoring" specific users,

SGTM -- FWIW, I made a similar choice in Restyled: it won't restyle PRs opened by bot users.

https://github.com/restyled-io/restyled.io/blob/8625ab2ac92d2fe8a6dd7b21832b9e5fa10175ac/src/Model/Repo.hs#L97-L101

This was originally to avoid a possible infinite loop, but I now kind of just like a simple "Don't run on bot PRs" policy.

voxpelli commented 5 years ago

I fully agree with this.

I want to have pull assigned assign all human-creates PR:s evenly, but not drown everyone in all of the bot PR:s from @renovatebot

Though: It would be nice to have pull assigner remove the proxy team that CODEOWNERS assigns for bot PR:s as well :)

voxpelli commented 5 years ago

@abinoda Any update on this? Did the solution you mentioned in https://github.com/pullreminders/backlog/issues/142#issuecomment-475826305 get implemented?

MysteryG commented 4 years ago

@abinoda Is there any timeline on when we would be able to ignore PR's created by a certain user as mentioned in your comment?