[core-workflow] self-approving pull requests

Nick Coghlan ncoghlan at gmail.com
Wed Feb 22 10:22:00 EST 2017


On 18 February 2017 at 09:24, Donald Stufft <donald at stufft.io> wrote:
> We turned on require code review for the PR, though at the time I *thought*
> it still allowed you to approve your own PR. Apparently that was wrong.
> Brett was the one to actually make the decision to do it and turned it on,
> so I don’t know if he knew that it didn’t allow people to self-approve or
> not. The impetus to do this was to make sure that all changes went through
> the PR process and folks didn’t directly push to `master`.

While this is a good ideal to strive for, I'm starting to think it
isn't practical with our current development set up.

The trigger for this change in my thinking was going to merge
https://github.com/python/cpython/pull/171 which is basically fine,
but needs a few clean-ups prior to committing:

- Misc/ACKS update
- Misc/NEWS entry
- What's New entry
- (and probably a rebase due to Misc/NEWS)

It's generally easier to just add those myself rather than asking the
contributor to do it, and then the contributor can look at the final
commit to see what these additional bookkeeping entries look like.

However, without the ability to commit locally and then push to GitHub
from there, you end up with one of two options:

- attempt to amend the original PR (see
https://github.com/python/devguide/issues/129 ). That may not work
depending on how the contributor has set up their fork and PR branch.
- create a new PR and close the original one (with the corresponding
subpar experience for the original contributor)

So as much as I'd like to keep it, it probably makes sense to turn off
branch protection for now and instead start making a list of the
things that would be needed to turn it back on, starting with:

- a way of reliably editing PRs that doesn't require closing them and
creating new ones
- conflict-free NEWS entry definition
- a way for core developers to handle minor changes autonomously
rather than always needing someone else to be online and responsive

Cheers,
Nick.

-- 
Nick Coghlan   |   ncoghlan at gmail.com   |   Brisbane, Australia


More information about the core-workflow mailing list