33 Explore and extend a pull request
Scenario: you maintain an R package on GitHub with pull requests (PRs) from external contributors e.g. Jane Doe, janedoe on GitHub. Sometimes you need to experiment with the PR in order to provide feedback or to decide whether or not to merge. Going further, sometimes you want to add a few commits and then merge. Or maybe there are just some merge conflicts that require your personal, local attention. Let’s also assume that you want the original PR author to get credit for their commits, i.e. you want to preserve history and provenance, not just diffs.
How do you checkout and possibly extend an external PR?
33.1 Update from the future
The lessons learned here eventually lead to the pr_*()
family of functions in usethis.
pr_fetch()
and pr_push()
are now my workhorses for exploring and extending PRs.
You can read more about usethis’s functions to help with pull requests in their very own article: Pull request helpers.
33.2 Terminology
Vocabulary I use throughout.
fork branch The name of the branch in the fork from which the PR was made. Best case scenario: informative name like fix-fluffy-bunny
. Worst case scenario: PR is from master
.
local PR branch The name of the local branch you’ll use to work with the PR. Best case scenario: can be same as fork branch. Worse case scenario: PR is from master
, so you must make up a new name based on something about the PR, e.g. pr-666
or janedoe-master
.
PR parent The SHA of the commit in the main repo that is the base for the PR.
PR remote The SSH or HTTPS URL for the fork from which the PR was made. Or the nickname of the remote, if you’ve bothered to set that up.
33.3 Official GitHub advice, Version 1
Every PR on GitHub has a link to “command line instructions” on how to merge the PR locally via command line Git. On this journey, there is a point at which you can pause and explore the PR locally.
Here are their steps with my vocabulary and some example commands:
-
Create and check out the local PR branch, anticipating its relationship to the fork branch. Template of the Git command, plus an example of how it looks under both naming scenarios:
# Template of the Git command git checkout -b LOCAL_PR_BRANCH master # How it looks under both naming scenarios git checkout -b fix-fluffy-bunny master git checkout -b janedoe-master master
-
Pull from the fork branch of the PR remote:
# Template of the Git command git pull REMOTE FORK_PR_BRANCH # How it looks under both naming scenarios git pull https://github.com/janedoe/yourpackage.git fix-fluffy-bunny git pull https://github.com/janedoe/yourpackage.git master
Satisfy yourself that all is well and you want to merge.
-
Checkout
master
:git checkout master
-
Merge the local PR branch into master with
--no-ff
, meaning “no fast forward merge”. This ensures you get a true merge commit, with two parents.# Template of the Git command git merge --no-ff LOCAL_PR_BRANCH # How it looks under both naming scenarios git merge --no-ff fix-fluffy-bunny git merge --no-ff janedoe-master
-
Push
master
to GitHub.git push origin master
What’s not to like? The parent commit of the local PR branch will almost certainly not be the parent commit of the fork PR branch, where the external contributor did their work. This often means you get merge conflicts in git pull
, which you’ll have to deal with ASAP. The older the PR, the more likely this is and the hairier the conflicts will be.
I would prefer to deal with the merge conflicts only after I’ve vetted the PR and to resolve the conflicts locally, not on GitHub. So I don’t use this exact workflow.
33.4 Official GitHub advice, Version 2
GitHub has another set of instructions: Checking out pull requests locally
It starts out by referring to the Version 1 instructions, but goes on to address an inactive pull request”, defined as a PR “whose owner has either stopped responding, or, more likely, has deleted their fork”.
This workflow may NOT give the original PR author credit (next time it’s easy to test this, I’ll update with a definitive answer). I’ve never used it verbatim because I’ve never had this exact problem re: deleted fork.
33.5 Official GitHub advice, Version 3
GitHub has yet another set of instructions: Committing changes to a pull request branch created from a fork
The page linked above explains all the pre-conditions, but the short version is that a maintainer can probably push new commits to a PR, effectively pushing commits to a fork. Strange, but true!
This set of instructions suggests that you clone the fork, checkout the branch from which the PR was made, make any commits you wish, and then push. Any new commits you make will appear in the PR. And then you could merge.
My main takeaway: maintainer can push to the branch of a fork associated with a PR.
33.6 A workflow I once used
The lessons learned here eventually lead to the pr_*()
family of functions in usethis.
pr_fetch()
and pr_push()
are now my workhorses for exploring and extending PRs.
You can read more about usethis’s functions to help with pull requests in their very own article: Pull request helpers.
This combines ideas from the three above approaches, but with a few tweaks. I am sketching this up in R code, with the hope of putting this into a function and package at some point. This is a revision of an earlier approach, based on feedback from Jim Hester.
Example of a PR from the master
branch (suboptimal but often happens) from fictional GitHub user abcde
on usethis.
library(git2r)
## add the pull requester's fork as a named remote
remote_add(name = "abcde", url = "git@github.com:abcde/usethis.git")
## fetch
fetch(name = "abcde")
## list remote branches and isolate the one I want
b <- branches(flags = "remote")
b <- b[["abcde/master"]]
## get the SHA of HEAD on this branch
sha <- branch_target(b)
## create local branch
branch_create(commit = lookup(sha = sha), name = "abcde-master")
## check it out
checkout(object = ".", branch = "abcde-master")
## set upstream tracking branch
branch_set_upstream(repository_head(), name = "abcde/master")
## confirm upstream tracking branch
branch_get_upstream(repository_head())
## make one or more commits here
## push to the branch in the fork and, therefore, into the PR
push()