Version 4
Created by jbalunas on Feb 15, 2012 4:40 PM. Last modified by jbalunas on Feb 28, 2012 10:33 AM.

    This article shows the guidelines and tasks required for project developers to manage and handle pull requests (PR).

    General Guidelines

    These are general guidelines about handling PRs and things to keep in mind as a project developer.

     

    • All changes should be made via PRs
    • You should not process your own PRs
      • Another project developer should handle your PR as described below
        • This is especially important for large or complex changes
      • Exceptions:
        • Updates are clearly trivial
        • No one is available to review in a timely manor
        • Updates are blocking your progress with other critical items
        • Another project developer has "+1"'ed the PR after a basic review
    • Do NOT use the GitHub PR merge facilities
      • This creates merge commits that we want to avoid

    Handling a Pull Request

    This section talks about handling a pull request generated from a community member or another project developer.

     

    Add a git remote reference to the PR location

    You will need to get the updates from the contributors repository into your own.  To do this create a remote reference to their repo

     

    git remote add contrib_usr git@github.com:[contrib_usr]/[target repo].git

    Create a topic branch and pull in PR updates

    It is highly recommended that you create a topic branch for verifying PRs.  After that you need to pull in their updates for review.

     

    git checkout -b <contrib_topic_branch>
git fetch <contrib_usr>
git rebase -i <contrib_usr>/<contrib_topic_branch>

     

    You now have a topic branch that should be the exact same as the PR.

    Verify, test, and comment on the PR as needed

    At this point you should:

    • Review all of the updates
    • Verify and test effected parts of the code
    • Check code style, and other standards

     

    Work with the contributor through comments in the PR, IRC, forums, or whatever will accomplish the task.  As the contributor makes updates work them until the PR is acceptable in quality and consistent with our coding standards.

    Push updates to upstream

    When satisfied with the quality and functionality of the PR you can push to the upstream repos.  Remember you must make sure you are up to date with upstream, and rebase as needed as described above.

     

    There are two options here for merging.  You can use --ff-only to not keep the topic branch - this is fine for simple, or single commit updates.  For complex or multi-commit updates you can use --no-ff which will force a merge commit, and maintain the topic branch.  This can make it easier to visualize, debug and bundle related commits.

     

    git checkout master 

#for simple or single commit updates
git merge --ff-only <contrib_topic_branch>

#for complex, or multi-commit update
git merge --no-ff <contrib_topic_branch>


git push upstream master
git push origin master
git branch -d <contrib_topic_branch>

     

    At this point the upstream repo, your fork, and your local repo's should be in sync and have the commits from the PR in place.

    Wrapping up

    The final steps are:

     

    • Close the PR
      • GitHub may do this for you automatically based on the push
    • Resolve the jira
      • In most cases it is best for the author to resolve the issue
        • Allows for the author to add any closing comments, or follow up info
      • If it is clear the issue was resolved the committer may resolve the issue
    • Thank the user, and offer to buy them a beer the next time we meet!

    Handling your own pull request

    This section describes addition steps you should take when handling your own pull request for the various reasons discussed above.  They are exactly the same and the instructions above, except as described outlined below.

    Test it again

    Your going to be committing code that has had only one set of eyes on it.  Run through a couple of extra tests, verify the wording, etc...

    Comment in the PR

    Add a comment in the PR describing why you needed to push this PR yourself.  If other project developers "+1"'ed this is not needed.