CCL | Software | Download | Manuals | Forum | Papers
CCL Home

Research

Software Community Operations

Integrating/Merging GitHub Pull Requests

Introduction

Now that we're receiving more contributor pull requests, it's important to integrate these into mainline properly. It's important to preserve the commit history of pull requests. Normally, this is simple and done for you when merging pull requests that do not need modified. GitHub has a "Merge This Pull Request" button which handles the merge properly. However, sometimes you will want to edit a pull request before merging it into the mainline. This guide will show you how to do that.

Setting Up Your Repository To Track GitHub Pull Requests

First, you want to be able check out the pull request in your local git repository. This is done quite simply since GitHub makes all pull requests available as references in the mainline. Follow this GitHub Help page to setup your repository to pull these references. If you followed the steps in Action Item #7 in the Git Introduction, then you already have these pull requests tracked:

$ git show-ref | grep pull 55f3d5bbe54a8ff050793717b41f1e0a312045aa refs/remotes/ccl/pull/1/head f8bd6455bd5c9d3eb2ae3368f92a7d03cd52739d refs/remotes/ccl/pull/1/merge [...]

Example Merged GitHub Pull Request

We will examine Pull Request #181.

The references for this pull request are:

$ git show-ref | grep pull/181 a0160e5c871d997d9dd11e9e9fb088d797fd4d45 refs/remotes/ccl/pull/181/head 4ddc755932d21fd6ad2deec2e1e0031ff90734c4 refs/remotes/ccl/pull/181/merge

Here the ccl remote is the CCTools mainline. The refs/remotes/ccl/pull/181/head has the pull request commits. refs/remotes/ccl/pull/181/merge is the same except for a merge commit done for you by GitHub. We won't use the merge reference.

Here is the commit graph for refs/remotes/ccl/pull/181/head:

$ git log --graph --abbrev-commit refs/remotes/ccl/pull/181/head * commit a0160e5 | Author: Nathanael Fillmore | Date: Tue Sep 3 10:32:16 2013 -0500 | | Fixed Python 3 detection bug in configure script. | * commit 97537ed | Author: Nathanael Fillmore | Date: Mon Sep 2 20:06:27 2013 -0500 | | Accomodate older versions of Python 3. | * commit acafc1a | Author: Nathanael Fillmore | Date: Mon Sep 2 19:03:51 2013 -0500 | | Added Python 3 support for work_queue library. | * commit 5626e74 |\ Merge: 86ede99 7f68369 | | Author: Dinesh Rajan | | Date: Mon Sep 2 12:31:23 2013 -0700 | | | | Merge pull request #172 from batrick/chirp-path-bugfix | | | | Chirp path bugfix | |

As you can see, Nathanael has added 3 commits.

We want to make some edits to this pull request before merging. First, check out the pull request in a new branch:

$ git checkout -b pull-181-merge ccl/pull/181/head Switched to a new branch 'pull-181-merge' $ git branch master * pull-181-merge $ git log -1 commit a0160e5c871d997d9dd11e9e9fb088d797fd4d45 Author: Nathanael Fillmore Date: Tue Sep 3 10:32:16 2013 -0500 Fixed Python 3 detection bug in configure script.

We will add two commits on top of this pull request, in our branch. These commits will be the same as those made to actually merge the pull request. The two commits are 6146cc0 and 919da6a. We add them:

$ git merge 6146cc0 Updating a0160e5..6146cc0 Fast-forward work_queue/src/python3/.gitignore | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) $ git merge 919da6a Updating 6146cc0..919da6a Fast-forward configure | 6 +++--- work_queue/src/python3/Makefile | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-)

So that this example mirrors what actually happened to merge the pull request, we cheated a little by pulling the commits directly, causing a fast forward, rather than manually making the edits.

We now consider the pull request ready to be merged into master. To do this, we begin by switching branches back to master. At the time of this pull request, master was 21351a3.

$ git checkout 21351a3 Note: checking out '21351a3'. You are in 'detached HEAD' state. You can look around, make experimental changes and commit them, and you can discard any commits you make in this state without impacting any branches by performing another checkout. If you want to create a new branch to retain commits you create, you may do so (now or later) by using -b with the checkout command again. Example: git checkout -b new_branch_name HEAD is now at 21351a3... Organize/alphabetize headers. Remove some repeats.

Of course, in an actual merge, instead of 21351a3, we would checkout master. To reproduce the example pull request merge, we used the commit identifier directly, causing our HEAD to become detached. A detached HEAD means that the current checkout is not referenced by a permanent ref/. In other words, it's a temporary workspace/checkout.

The final step is to merge our branch pull-181-merge into 21351a3 (what was master at the time):

$ git merge pull-181-merge -m 'Merge #181.' Merge made by the 'recursive' strategy. configure | 101 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ work_queue/src/python3/.gitignore | 6 ++++++ work_queue/src/python3/Makefile | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 155 insertions(+) create mode 100644 work_queue/src/python3/.gitignore create mode 100644 work_queue/src/python3/Makefile

This gives commit (note the commit identifier will be different for you):

$ git log -1 commit 03bd1e96a6fc9b3394894c6724472309582c6fea Merge: 21351a3 919da6a Author: Patrick Donnelly Date: Mon Sep 16 21:35:24 2013 -0400 Merge #181.

The actual merge commit for the pull request was fe84283.

The commit graph is below. Notice how the entire pull request follows a single branch, even our edit commits 6146cc0 and 919da6a. This is important. Why? Because these edit commits could be correcting serious bugs. It is best that master never refer to buggy commits. It is also logically useful that all changes a pull request makes follow a single branch.

$ git log --graph --abbrev-commit * commit 03bd1e9 |\ Merge: 21351a3 919da6a | | Author: Patrick Donnelly | | Date: Mon Sep 16 21:35:24 2013 -0400 | | | | Merge #181. | | | * commit 919da6a | | Author: Patrick Donnelly | | Date: Wed Sep 4 15:59:34 2013 -0400 | | | | whitespace | | | * commit 6146cc0 | | Author: Patrick Donnelly | | Date: Wed Sep 4 15:59:11 2013 -0400 | | | | alphabetize/simplify | | | * commit a0160e5 | | Author: Nathanael Fillmore | | Date: Tue Sep 3 10:32:16 2013 -0500 | | | | Fixed Python 3 detection bug in configure script. | | | * commit 97537ed | | Author: Nathanael Fillmore | | Date: Mon Sep 2 20:06:27 2013 -0500 | | | | Accomodate older versions of Python 3. | | | * commit acafc1a | | Author: Nathanael Fillmore | | Date: Mon Sep 2 19:03:51 2013 -0500 | | | | Added Python 3 support for work_queue library. | | * | commit 21351a3 |/ Author: Patrick Donnelly | Date: Tue Sep 3 17:05:19 2013 -0400 | | Organize/alphabetize headers. Remove some repeats. | * commit 5626e74 |\ Merge: 86ede99 7f68369 | | Author: Dinesh Rajan | | Date: Mon Sep 2 12:31:23 2013 -0700 | | | | Merge pull request #172 from batrick/chirp-path-bugfix | | | | Chirp path bugfix | |

Also notice the intervening commit 21351a3. This prevented the merge from being a fast-forward.

Take Aways

  • Preserve the commit history of pull requests. Do not rebase, squash, or manually patch pull request commits. If the commit identifier for the contributor's commit has changed, you probably messed up!
  • Edit commits should be made in a branch. Do not merge the pull request and then apply fixes.