Merging Pull Requests

David E. Wheeler edited this page Nov 5, 2010 · 2 revisions
Clone this wiki locally

Merging Pull Requests

First, you'll need to fork the Bricolage repository. "Working with Git" has the details on that.

There are two basic ways to merge in a pull request: Via a fast-forward merge or by cherry-picking. The former is preferred, because it allows the requestor's original commit(s) to be merged in identically, so that even the sha1 is the same. But that really only works if the commit(s) to be merged in come after the latest merge in the repository into which they're being merged.

To determine which will work, it's usually simplest to try the merge, and if it's a fast-forward, then great, you're done. Let's try that first.

Review

Before you do anything else, read the pull request and look at its diff. Make sure that it's something you actually want to merge into the Bricolage core. The merge should:

  • Contain an appropriate bug fix, improvement, or new feature
  • The included code should meet the project's Coding-Standards
  • If the change is to the core Perl libraries, there should be tests to make sure the bug stays fixed, or to test the new feature or improvement
  • There should be a note in Bric::Changes about the change (although you can add one yourself after the merge, which might be simpler, anyway).

If you need more from the requestor, ask for it. She'll either give you want you need and it will show up in the pull request, or she'll send a new pull request. Either way, be sure the code is ready for the project before you pull it in.

A Fast-Forward Merge

In this section, I'll be merging this pull request into Bricolage. If you consult an open pull request, you'll see that the bottom of the pull request page will have instructions for how to do a merge. In this case, the instructions said to do this (with more explanation here):

  • cd into a local clone of your fork of the Bricolage repository and make sure it's up-to-date, from both your fork on github and the canonical upstream repository:

    git checkout master
    git pull origin master
    git pull upstream master
    
  • Pull from the requestor's branch. In this case, the requestor had pushed one commit to the "master" branch of his fork. Oftentimes the commit will come from some other branch (usually a working branch used by the requestor only for the pull request). Either way, the pull request will tell you which branch to merge from, and which branch the requestor wants you to merge into. So here, we're going to merge from brewt's master branch into our own master branch. Start by pulling brewt's master branch:

    git checkout -b brewt-master master
    git pull https://github.com/brewt/bricolage.git master
    
  • Then merge the change into your local master branch

    git checkout master
    git merge brewt-master
    
  • Now you have pulled in the changes. At this point, git will tell you if you've done a fast-forward commit. If you've somehow missed that, run git log. If the most recent commit is a merge commit, you don't want it. In such a case, you'll want to throw out the merge commit. You can do so like this:

    git reset --hard HEAD^
    

    That command will delete the most recent commit. Skip to the next section to do a cherry-pick, instead.

  • Run the test suite:

    perl Makefile.PL
    sudo make dev
    sudo make devtest
    
  • If the tests have passed, great! Make sure that Bric::Changes has the necessary note(s), and if the contributor is not listed in Bric::License or comp/widgets/help/patchers.html, add them! Commit those changes, and push it to GitHub:

    git push origin master
    
  • If you have permission to push to the canonical Bricoleurs repository, push it there, too:

    git push upstream master
    
  • And finally, drop the copy of the requestor's working branch:

    git branch -d brewt-master
    
  • Back patch if necessary. See below for more information.

Cherry-Picking

If a fast-forward merge fails, you'll need to cherry-pick the requested commit(s). This is similar to applying a patch with patch, but it will also commit the commit(s) with the requestor's original log message(s). This is almost the same as a fast-forward, except that the SHA1 will be different than in the requestor's commit. This won't be a big deal for you or for the Bricolage project, but if the requestor hasn't created a working branch, she might have to do some fiddling with her fork to get it back in sync with upstream. This is why it pays to use [working branches]Working-with-Branches for all work.

Here's how to cherry-pick the changes:

  • Add the requesting repository as a remote:

    git remote add brewt https://github.com/brewt/bricolage.git
    
  • Look at the pull request to see what commits you want to pull in. In this case, there's just one, with the sha1 "2311662c9e68d69cc77b". Cherry-pick that:

    git cherry-pick 2311662c9e68d69cc77b
    
  • If there are any conflicts, [merge them]Merging-with-Git and commit. Then run the test suite as usual, push to your repository and upstream, and pack-patch as required (see next section).

Back-Patching

Many pull requests are for bug fixes that apply not only to the next major release of Bricolage, which is managed in the master branch, but also should be back-ported to one or more maintenance branches. The above merged patch, for example, fixes a bug that affects both 2.0 and 1.10. So it needs to be back-ported to those branches.

  • Run git log and copy the sha1 of the just-merged commit or commits.

  • Check out the rev-2.0 branch.

    git checkout rev-2.0
    

    If you don't have a rev-2.0 branch, create one, and make sure it tracks the upstream branch with the same name:

    git checkout -b rev-2.0 origin/rev-2.0
    
  • Make sure the branch is up-to-date:

    git pull origin rev-2.0
    git pull upstream rev-2.0
    
  • Cherry-pick the merge or merges:

    git cherry-pick 2311662c9e68d69cc77b
    
  • If you have merge conflicts (and you probably will in Bric::Changes), [fix them]Merging-with-Git and then commit.

  • Run the test suite as usual, then, assuming all is good, push the commits to your repository and, if you have permission, to the upstream repository:

    git push origin rev-2.0
    git push upstream rev-2.0
    
  • If the patch is appropriate for the rev_1_10 branch, repeat these steps to pack-patch it t that branch, too.

A Final Note

Back-patching to maintenance branches is, workflow-wise, the same as cherry-picking into the master branch. But not all merge reqeusts will be for the master branch. Sometimes they will apply only to the rev-2.0 branch, or just rev_1_10, and so you might be able to fast-forward merge them into the appropriate branch. The key is to pull things in as best you can, add any appropriate notes to Bric::Changes and the two contributor credit files, and cherry-pick into other branches as appropriate.