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

Autolabel PR's that are ready to land? #33164

Closed
ronag opened this issue Apr 30, 2020 · 7 comments
Closed

Autolabel PR's that are ready to land? #33164

ronag opened this issue Apr 30, 2020 · 7 comments
Labels
meta Issues and PRs related to the general management of the project.

Comments

@ronag
Copy link
Member

ronag commented Apr 30, 2020

We already have a functionality in node land which check whether a PR can land or not, i.e. sufficient time and approvals etc. Would it be possible to create a bot that automatically checks this for PR`s and applies/removes a label accordingly on GitHub?

Might be helpful when going through PR's and trying to land them. Or what do you think @Trott @BridgeAR?

@legendecas
Copy link
Member

legendecas commented Apr 30, 2020

Discussions have been taken on nodejs/build#2201 recently.

@richardlau
Copy link
Member

It cannot be done in GitHub actions as they don't allow write permissions to add labels to PR's from forks.

The Node.js GitHub bot (https://github.com/nodejs/github-bot) does currently automatically apply labels on PR create.

@mscdex
Copy link
Contributor

mscdex commented Apr 30, 2020

I'm not sure something like that is feasible. For example, there are many times when I've seen people "request" one or more changes to be made, but click "approve" early with the assumption the changes will be made. There could also be concerns raised in the PR (not via the "request changes" mechanism) that have yet to be addressed.

@Trott
Copy link
Member

Trott commented Apr 30, 2020

FWIW, as a rudimentary substitute for something like this, I used to search for PRs that were open and approved in this repository, omitting certain labels (like the work-in-progress one). Then scroll down to ones that say 3 days or older (or reverse the sort order to be oldest first).

Something like this: https://github.com/nodejs/node/pulls?page=4&q=is%3Apr+is%3Aopen+review%3Aapproved+-label%3Asemver-major+-label%3A%22work+in+progress+%28WIP%29%22+-label%3Ablocked

@BridgeAR
Copy link
Member

@ronag it would definitely be possible to create such a bot but it has to know about a lot of special cases:

  • No open comment
  • Full CI is started after the last commit or it's a documentation only change
  • At least one LG non-semver-major PRs and two for semver-major ones
  • No "blocked", "WIP" or "pending" label
  • It's not a draft

As @mscdex pointed out it won't be possible to catch requests for changes that are just regular comments that way but it should be sufficient to at least mark most PRs correct. I think as soon as such bot would be active, people would pay more attention how to ask for changes to prevent the bot from labeling it wrong.

@targos
Copy link
Member

targos commented Dec 27, 2020

@ronag now that we have the commit queue, do you think we need to keep this open?

@targos targos added the meta Issues and PRs related to the general management of the project. label Dec 27, 2020
@Trott
Copy link
Member

Trott commented Jan 6, 2021

I'm going to go ahead and close this, but absolutely feel free to re-open and/or comment if you think that's the wrong decision. Just tidying up a bit, but not acting on a strong conviction or anything like that.

@Trott Trott closed this as completed Jan 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Issues and PRs related to the general management of the project.
Projects
None yet
Development

No branches or pull requests

7 participants