Should successful backport PRs by miss-islington be automatically approved?


#1

At the moment core devs still need to approve backport PRs created by miss-islington.
I’m wondering if we can reduce this step, so backport PRs (by miss-islington) are automatically considered “approved” so we don’t even need to manually approve it? (i.e. just apply the “awaiting merge” label)

  • yes, auto-approve the backport PR
  • no, core devs need to approve

0 voters


#2

Edited with a poll.
Poll is private.


(Victor Stinner) #3

They are multiple reasons why backports must not be merged automatically:

  • Sometimes, “needs 3.6 backport” label is added, whereas it’s a very bad idea: like a new feature which should not be backported, or a corner case bugfix which risks to introduce a regression
  • Sometimes, it’s better to give buildbots time to ensure that the change pass. Whenever possible, it’s better to not add a regression to 3 branches at once :slight_smile: (Hopefully, regressions are becoming less and less frequent!)

More generally, a human validation doesn’t hurt.

Approving with “LGTM, good bot!” takes 5 seconds. IMHO it’s not a big deal.


#4

Yeah, I guess I’ve dealt mainly with documentation backport PRs, and I’m starting to find it a hassle having to approve the same PRs again. For documentation only changes, likely if the PR backport cleanly, it indicates everything was ok. If it was a mistake, say the text didn’t exist in the branch, then there will be conflict and the backport wouldn’t work.


(Victor Stinner) #5

I would be fine to automatically merge a documentation backport as soon as the CI pass. You have to define what is a “documentation change”. Maybe a change only impacting “*.rst” files in “Doc/”?


#6

Documentation only change in my mind is PR that contains only .rst files. So if a PR contains other file types (mix of .py and .rst) then it doesn’t count.

Also bedevere already automatically labeled those PRs with type-documentation label (thanks @csabella for implementing this!) , so we can relying on the type-documentation label to identify that this is documentation only PR.


(Antoine Pitrou) #7

Is “approving” backports a new thing? I would simply press the merge button once CI passes and I’ve checked the diff is ok.


#8

If you approve the PR, miss-islington will automatically merge so you don’t have to wait for CI to pass.


#9

And it’s not new thing, it’s been done since Feb 2018 https://mail.python.org/pipermail/python-committers/2018-February/005243.html


#10

Thought I’ll be clear, I’m only referring to auto-approving the backports made by miss-islington. Other types of backports still require review.


(Antoine Pitrou) #11

Ah, thank you. I don’t make many PRs, so I usually wait for CI anyway :slight_smile: