search

nodejs / github-bot

@nodejs-github-bot's heart and soul
MIT License
264 stars 113 forks source link

Which backport PR labels should be added by the bot? #120

Open phillipj opened 7 years ago

phillipj commented 7 years ago

Moving an interesting discussion ignited in https://github.com/nodejs/github-bot/issues/116 into its own issue.

Which labels should the bot automatically add when it attempts backport of PRs?

First a description of the current auto labelling logic, so we're all on the same page when discussing how backporting labels should work as a whole.

Backport attempt fails

PR patch does not land cleanly against a staging branch.

If it is a LTS staging branch dont-land-on-v${version}.x is added*, otherwise a previously added lts-watch-v${version}.x might be removed as long as the user added the watch label was the github-bot.

Backport attempt succeeds

PR patch lands cleanly against a staging branch.

If its a LTS staging branch lts-watch-v${version}.x is added, otherwise a previously added dont-land-on-v${version}.x is removed if the user who added the dont-label label was the github-bot.

Introduce explicit auto labels?

In https://github.com/nodejs/github-bot/issues/116 there were several questions and concerns related to the dont-land-on-* labels especially. Those labels are used by devs deciding what should go into staging branches, to definitely stop any unwanted PRs (described in https://github.com/nodejs/github-bot/pull/90#issuecomment-261095822 and https://github.com/nodejs/github-bot/issues/116#issuecomment-275544912). There has been raised concerns about those hard stop labels automatically, since the bot adding that label currently means it does not land cleanly, which it sounds is not the real intention of dont-land-on-* labels.

There has previously been suggested introducing explicit auto labels for these automatic backport attempts, such as auto-merge-to-v7.x-failed or similar as described in https://github.com/nodejs/github-bot/issues/116#issuecomment-275181198.

Who is these auto labels intended for?

In addition to exactly which labels the bot should add based on these backport attempts, it seems to be some confusion about who these labels are intended for. The PR author or devs staging for releases?

* dont-land-on-* labelling has recently been temporary disabled: https://github.com/nodejs/github-bot/pull/118

MylesBorins commented 7 years ago

Don't have time to go through all of it right now but I do want to request that we do not auto land any do-not-land labels to any branches unless it is for specific features that are blacklisted (for example the inspector for v4)

edit: Also have we been auto landing Do-not-land for LTS? I thought we explicitly did not do this. If we have been doing it we will need to do a complete audit of commits to make sure we didn't miss backporting stuff

edit 2: https://github.com/nodejs/github-bot/pull/118/files#diff-eea09a4f0f8288ff9bd49ef44625f0c1R134

So we were only putting that label for non-lts... no need to audit :D. That being said we may want to audit the backlog for v7 to make sure things were not missed