bot to automatically close PRs that merge maintenance branch into master

[Mariatta]

I've closed a number of PRs where contributor tries to merge one of the maintenance branches into master:
(e.g. 3.6 into master or 3.5 into master)

python/cpython#2901
python/cpython#2784
python/cpython#2678
python/cpython#2427
python/cpython#1968

Would be great if somehow we can disallow this type of PR, or automatically close the PR once created.

From GitHub Web UI, each of these PRs are showing maybe hundreds of commits.
It's inconvenient to scroll all the way to the bottom to find the Close button 😞

[brettcannon]

Looking at https://api.github.com/repos/python/cpython/pulls/2901 maybe if we checked head/ref and base/ref to make sure they both aren't master or \d+.\d+? Something I'm not sure about, though, is what if someone forks and simply commits into their e.g. master branch and creates a PR from that?

[zware]

I think the main issue here is a GitHub bug; it should not be possible for a non-team member to click the "New pull request" button on the branches page. As a defense against that, we could check that head.label and base.label don't both match python:(master|\d+.\d+).

[brettcannon]

@zware just for my own edification, what exactly does *.label represent and why does having the python bit guarantee it's only those branches off of the cpython repo and not someone who just edited master on their personal fork?

[zware]

@brettcannon *.label is just something I picked out from the api.github.com link for 2901 that you included above; it seems to be basically f'{head.repo.owner.login}:{ref}'. So in 2901, head.label was python:3.6 and base.label is python:master, while in 18 (a legitimate PR) head.label was rogersachan:master with base.label python:master. You could also directly check whether head.repo.owner.login is python and reject the PR if head.ref matches ^(master|\d+.\d+)$ since we don't do merges between branches anymore.

(Sorry for the delay in answer, I'm still in the midst of moving house.)

[brettcannon]

@zware thanks for the info!

And no need to apologize! This is not exactly holding anything up and real life always takes precedence over volunteer life. Hope the move is going smoothly!

[brettcannon]

So it would seem if a PR has a head.label matching python:(master|\d+\.\d+((a|b|rc)\d+)?) that's good enough and has simpler logic than having to check both login and ref. Does that sound reasonable?

[zware]

I'm not sure about the ((a|b|rc)\d+)? part; we may want some release manager input on that (do release managers create branches in the main repo during the release process that then get merged?). We should probably also explicitly match beginning and end of string, so that we don't reject something like ilikepython:3.6-backport-of-some-issue.

Otherwise, sounds good to me :)

[brettcannon]

So ^python:(master|\d+\.\d+)$? I guess it's easier to just not worry about alphas and such and assume people won't slip up like they have with the mainline branches.

[Mariatta]

Happened twice yesterday:
python/cpython#3455
python/cpython#3449
We have codeowners now, I'm guessing many people are getting spammed unnecessarily with this type of PRs 😞

[terryjreedy]

Both of those PRs are from people who have not even forked cpython. I think that they should be reported to github support (button on their profile pages) as we may not be the only target of such abuse. Let GH support investigate their history and decide on appropriate action.

[zware]

I've just sent a message to GH support about it.

[Mariatta]

Thanks @terryjreedy @zware

[zware]

I've had a response from GH support; they're investigating those two particular users and have relayed my feedback on removing the 'new pull request' button for those with no write access.

[zware]

I've also mentioned python/cpython#3621 to GH support, and asked for an update on removing the button.

[zware]

Just reported python/cpython#3689 as well. No news on a fix, but my "feedback is being discussed internally".