python / core-workflow

Issue tracker for CPython's workflow
https://mail.python.org/mailman/listinfo/core-workflow
Apache License 2.0
95 stars 59 forks source link

Automatically apply "needs backport to " labels. #267

Open Mariatta opened 6 years ago

Mariatta commented 6 years ago

One of the core developer's chores is determining if a PR needs to be backported, and then actually apply the labels.

I'm thinking that perhaps we can delegate this task to the PR author.

Currently we already utilize the PR template in CPython.

We can update the PR template to include metadata:

Needs backport to: 2.7 | 3.6 | 3.7

If we can let PR author to do their own research and determine where to backport to and give the above information, bedevere can then apply the necessary labels.

Core devs can still override that by removing the labels later on.

brettcannon commented 6 years ago

I'm personally on the fence for this one. A core dev is still going to have to reason through whether a backport is necessary, so while this would save adding the labels, that also makes it a conscious decision as to whether it should be backported.

Then again, doing this does empower external contributors a bit more while still having core devs act as gatekeepers.

terryjreedy commented 6 years ago

Given that contributors submit PRs faster than core-developers can review, revise, and reject or merge, my concern is whether a change saves core-dev time on net. (And faster disposition is also a benefit to contributors.) Tracker experience makes me wary about letting just anyone set labels on submitted PRs (in particular, after a core-dev has set them).

On the other hand, I recently noticed that the PR creation form has a labels button, and I find it nicer, somehow, to add them before submitting. Are non-core-developers currently prevented from using the labels button when preparing a PR? If so, can we change this? I would be in favor of trying this.

Mariatta commented 6 years ago

Only core developers can add labels when preparing the PR, we can't change that, but we can make our bots do it. So my idea is to only detect the Needs backport to metadata once, when they open the PR. After that, even if they edit, the bot won't update the labels. This will prevent re-labeling from non core-devs.

terryjreedy commented 6 years ago

I am not sure what metadate you mean. The Versions selected on the associated tracker issue? Since that should be opened first, and I always mark the version, I would like that.

Mariatta commented 6 years ago

I would actually like it if we can automatically add "needs backport to X.Y" based on what's selected in the bpo. But I don't know if this is possible, and how to do that.

By metadata, I'm referring to my proposal of adding the following in the pull request template, and let the PR author fill in the appropriate versions.

Needs backport to: X.Y
terryjreedy commented 6 years ago

When I follow the PR Template link above and click on [raw], I see the boilerplate text in the comment box under the title. I routinely delete it before adding a real comment. I assume that you cannot put buttons within the text. Having users type in 'Needs backport to: 3.7, 3.6' probably won't work well.

Getting the data from the tracker database seems better.

https://bugs.python.org/issue?%40search_text=&ignore=file%3Acontent&title=&id=33855&%40columns=id&stage=&creation=&creator=&activity=&%40sort=activity&actor=&nosy=&type=&components=&versions=&%40columns=versions&dependencies=&assignee=&keywords=&priority=&status=1&resolution=&nosy_count=&message_count=&%40group=&%40pagesize=50&%40startwith=0&%40sortdir=on&%40queryname=&%40old-queryname=&%40action=search

returns a web page containing 33855 | Python 3.8, Python 3.7, Python 3.6.

https://bugs.python.org/issue?@action=export_csv&@columns=id,versions&@filter=id&id=33855

returns the same data in a two line (header + data) csv file without the unwanted web stuff. The versions are listed as [20, 21, 22], 2.7 is 14.

Mariatta commented 6 years ago

The current PR template is inside a comment tag. You can leave it as is, and it will not be displayed. You don't have to delete it.

The idea is to add another line on top of the current comment tag, and not inside the comment tag. It will be prefilled Needs backport to: 2.7 | 3.6 | 3.7 (whichever versions we still maintain). Then PR author just need to remove ones that don't apply. If none apply, then remove the entire line.

I find deleting is easier than typing things out.

The web page result from bpo is not useful to the bot. The csv, maybe...

brettcannon commented 6 years ago

I think this is really going to come down to whether it's less work to remove misclassifications of backport labels compared to actively adding backport labels. And I don't know if we will know that until this is actually implemented. And if we're going to do that then I think Mariatta's idea in Python/bugs.python.org#11 makes sense as the way to do this to help get issue triagers involved as well.

maxking commented 5 years ago

Could we instead maintain a static list of currently maintained stable and security versions and automatically add "needs backport to " labels based on that list to "type-bugfix" and "type- security" labels? This is much simpler logic IMHO.

I am not sure about the situations where one would want a bugfix to not be backported, because of a behavioural change I guess?

This brings down the maintenance burden down to just tagging PRs with type-bugfix and type-security which I see happens often (not always though). I also see much less errors with this, except for the fact that the static list would need to be a changed every 18months when a new Python version comes out (and when a version is retired), which could perhaps be a part of Release Manager runbook (if that exists, I am not sure about it :).

Mariatta commented 5 years ago

Not all bugs need to be backported all the way to the earliest maintained version. I know from recent examples where some docs/bug fixes appearing in 3.8 but not 3.7, so we only needed to backport to 3.8 branch, but not 3.7 branch.

ezio-melotti commented 2 years ago

Currently we also have 3.* that could be used to mark issues/PRs, and let the core dev use them to decide which need backport to 3.* label to add.

Could we instead maintain a static list of currently maintained stable and security versions and automatically add "needs backport to " labels based on that list to "type-bugfix" and "type- security" labels? This is much simpler logic IMHO.

This was also proposed and discussed here: https://discuss.python.org/t/github-issues-migration-is-coming-soon/13791/47