Reviving / Carrying Forward PRs

[jdevries3133]

The Problem: Abandoned PRs

I am wondering about whether it is helpful for contributors to help move forward pull requests where the original contributor seems to have lost momentum for whatever reason. For example, I found bpo-19217 and PR-41203. There had been a long discussion and extensive review on the PR, but the original contributor stopped just short of making some final changes needed for merging. Of course, open source contributors have no obligation to continue working on a pull request, and the review process can definitely be challenging. At the same time, I notice so much core developer time being sunk into pull requests that never land. Some for good reason, but others because the original contributor just can't keep following up.

When I came across this issue, I went ahead and merged the original contributor's changes into a fresh branch on my fork, and did the additional work needed to create a branch ready for merge and to close the bpo, but I stopped short of opening a pull request because I didn't know what the procedure was and I definitely didn't want to step on the original contributor's toes. My changes can be viewed here.

Anyway, I am wondering what we think about this situation. Is there a role for contributors to help push forward pull requests that have lost steam? How can we make sure that both committers are equally recognized for their contribution? Pesumably only one can end up in the commit log, otherwise there will be unstable commits in cpython:main. In an ideal world, this workflow wouldn't be necessary, but I notice that there is so much great work lingering in 80%-complete github pull requests. This also wastes a ton of core dev time, because they review things that don't end up landing when the original contributor can't keep following through.

Role of the Dev Guide

I think that the dev guide can help here by establishing guidelines and procedures for people to do this type of work. For example, this chapter of the dev guide could:

• Establish a procedure for "passing the torch"
  • After n number of days of no response from the original contributor, it's fair game for another contributor to merge changes into their own branch and start carrying the issue forward
  • Or, after n number of days, a new contributor can request to take over with a comment on the bpo or PR. The original contributor has n additional days to respond, before it is ok for the new contributor to finish their work.
• Include a tutorial for how to take over:

git remote add original-contributor [email protected]/<other-contributor>/cpython
git merge other/<bpo-####>

• Explain how both contributors will receive credit.

[iritkatriel]

Sharing credit is done via the "Co-Authored By" tag. There is some relevant info here: https://devguide.python.org/pullrequest/?highlight=co-authored#converting-an-existing-patch-from-b-p-o-to-github

[jdevries3133]

@iritkatriel That's a great point. Do you think it would be a good idea to create a bot or github workflow that applies a Is Co-Authored (or similar) tag to commits where the contributor's PR includes commits that they did not author?

Or, are those tools not necessary, and it's clear from a core-dev point of view when co-authoring happens, and the risk of accidentally not giving co-author credit in this way is insignificant?

[encukou]

GitHub adds Co-Authored-By when squashing a PR with multiple authors. I'm pretty sure Miss Islington (the auto-merge bot) does as well. So as long as you don't squash/rewrite the commits, that risk is small.

As for taking over commits, I'd say that as long as you are sure enough that you're not doing unnecessary duplicate work, go ahead. Leave a courteous message like "I did the requested changes at #1234. @previous-committer I hope you don't mind!"

As a core dev, if I happen to have the time I'll put on the finishing touches myself and merge – it's often faster than describing the changes as review comments. It's not that different if another contributor does it, except you need to open a new PR. The exception is if it looks like the contributor wants more to learn the process rather than get a fix in. But if the PR is stalled for a few weeks and there's no note of going on vacation, there's little chance of that.

[encukou]

Oh, and: You're clearly thinking about how the other people involved will feel. I think that's more important than a ~~documented~~ rigid process :)

[iritkatriel]

As a core dev, if I happen to have the time I'll put on the finishing touches myself and merge

Though in that case we typically remove the automated Co-Authored-By so that we are not included as co-authors on the commit.

[jdevries3133]

GitHub adds Co-Authored-By when squashing a PR with multiple authors

Ok, that is good to know and exactly what I was wondering!

You're clearly thinking about how the other people involved will feel. I think that's more important than a documented rigid process

I agree. There's no need to make it rigid or to make "rules," but if the feeling is that these types of contributions are helpful, there is plenty of this work to go around once we teach people the workflow. By browsing random issues on the bug tracker, you can find an almost endless stream of cases like this. I can't imagine how frustrating it is as a core dev to put all the work into reviews on pull requests that go stale.

I'd like to see something added to the dev guide to this effect. Maybe we could add it to the bottom of the "easy issues" page. I think it's related in the sense that we are trying to provide contributors ways to help:

# Random Issues & Reviving Pull Requests

In the bpo sidebar, you will see a "random issue" option. In addition to the
"easy issues" tab, this can be a good way to find issues that never got much
attention, or that lost the attention of those working on it before it could be
merged. As a contributor, reading through the conversation around one of these
issues, figuring out how the issue can be resolved, and doing some work to
create a new and viable pull request is helpful towards clearing the backlog of
open issues.

 ## How to Revive or Carry Forward an Unfinished Pull Request

As you review open issues, you might find the following scenario: You notice
that there is a pull request attached to the bug report. You open the PR and
read through the discussion and changes. It's almost an acceptable fix, but
there are a few tests to fix, documentation to cleanup, or changes to make. It
may have even been reviewed by a core developer and received specific feedback
on what needs to be done next. However, the trail goes cold and there has been
no word from the original contributor in a *long* time. It's a touchy
situation, but there might be an opportunity for you to help!

Before going any further, it would be prudent to check in with the original
contributor. Leave a comment on the PR letting the original contributor know
that you are interested in continuing their work, or preparing it to be merged.
Remember that even if the PR appears small, the original contributor may have
put a lot of effort into researching the problem, identifying the fix, and
writing the contributions that you see. They have every right to insist that
they continue working on the issue if they wish. If the issue is urgent, it is
the responsibility of Core Developers to manage its swift resolution, so any
abandoned issue like the ones being discussed here are, by definition, not
urgent.

If you receive no response, or the original contributor is ok with you picking
up where they left off, you can merge their commits into your branch by
following these directions:

    git checkout -b bpo-####
    git remote add first-contributor <url to original contributor's fork of cpython>
    git fetch first-contributor
    git merge first-contributor/<branch name>

This is just a rough draft. I'll refine it and open a pull request if you want to discuss the changes in detail, but what do you think about adding content to this effect, in general?