summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRyan Tomayko <rtomayko@gmail.com>2010-08-31 09:31:36 -0700
committerRyan Tomayko <rtomayko@gmail.com>2010-08-31 09:31:52 -0700
commit899cdb6a8152c6752036856882ad32da17a5f521 (patch)
tree3f9cdf45470ddcf4c309b4faec6e977ca21ed5e4
parentc23fb9bed7a6a6f87fbdb1d5450d9f658e2da043 (diff)
new school
-rw-r--r--_posts/2010-08-29-pull-requests.md262
1 files changed, 187 insertions, 75 deletions
diff --git a/_posts/2010-08-29-pull-requests.md b/_posts/2010-08-29-pull-requests.md
index fab7bd7..6453331 100644
--- a/_posts/2010-08-29-pull-requests.md
+++ b/_posts/2010-08-29-pull-requests.md
@@ -6,117 +6,229 @@ categories: collaborating
main_category: collaborating
---
-Pull requests are a way to poke someone and let them know you've got some code
-they may want.
+<style type="text/css">
+.main img {
+ border:1px solid #eee;
+ padding:5px;
+}
+.main img.inline {
+ vertical-align:middle;
+ border:0;
+}
+</style>
-They work great when you want to let an upstream maintainer know that you've
-pushed some useful changes to your fork of their project.
+Pull requests let you tell others about changes you've pushed to a GitHub
+repository. Once a pull request is sent, interested parties can review the set
+of changes, discuss potential modifications, and even push follow-up commits if
+necessary.
-They also work great if you want to let a fellow developer know that you've
-pushed some experimental features to an arbitrary branch.
+This guide walks through the process of sending a hypothetical pull request and
+using the various code review and management tools to take the change to
+completion.
-### Step One
+## A Quick Note on Collaborative Development Models
-First, navigate to your project with the changes you want someone else to pull.
-In this instance, I'm going to go to
-"defunkt/grit":http://github.com/defunkt/grit and tell mojombo to pull a change.
+There are two popular models of collaborative development on GitHub:
-<img src="http://img.skitch.com/20080223-xw8wdwkg9abpjtuwu4wutwwcqm.png"/>
+ 1. The *Fork + Pull Model* lets anyone fork an existing repository and
+ push changes to their personal fork without requiring access be granted
+ to the source repository. The changes must then be pulled into the source
+ repository by the project maintainer. This model reduces the amount of
+ friction for new contributors and is popular with open source projects
+ because it allows people to work independently without upfront
+ coordination.
-### Step Two
+ 2. The *Shared Repository Model* is more prevalent with small teams and
+ organizations collaborating on private projects. Everyone is granted push
+ access to a single shared repository and topic branches are used to isolate
+ changes.
-I can type in a message if I want and select any number of recipients. By
-default, I'll see everyone who has a project in my project's network. If I want
-to add someone else, I can use the autocompleter and click 'add.'
+Pull requests are especially useful in the *Fork + Pull Model* because they
+provide a way to notify project maintainers about changes in your fork. However,
+they're also useful in the *Shared Repository Model* where they're used to
+initiate code review and general discussion about a set of changes before being
+merged into a mainline branch.
-<img src="http://img.skitch.com/20080223-1njj3f5559un2eyt51j92t2xkf.png"/>
+## Before You Begin
-### Step Three
+This guide assumes that [you have a GitHub account](http://github.com/signup),
+that you've forked an existing repository and pushed your changes. For help with
+forking and pushing changes, see the [Forking a project](/forking/) topic.
-Sent!
+## Initiating The Pull Request
-<img src="http://img.skitch.com/20080223-8jf2dc17cdtibs2jt13ptgxwts.png"/>
+In the following example, **kneath** has completed some work on an error page
+for the GitHub Jobs web application, pushed three commits to a topic branch in
+his fork, and would like someone to review and merge.
-### Step Four
+Navigate to your repository with the changes you want someone else to pull and
+press the *Pull Request* button.
-When mojombo logs in, he'll see that there's a new message waiting for him.
+![](http://img.skitch.com/20100831-qfk1c9wyt89pfgfxg61bh1r8rn.png)
-<img src="http://img.skitch.com/20080223-rpgh5c4e1n38wc3e45cacq77ww.png"/>
+Pull requests can be sent from any branch or commit but it's recommended that a
+topic branch be used so that follow-up commits can be pushed to update the pull
+request if necessary.
-### Step Five
+## Previewing The Pull Request
-<img src="http://img.skitch.com/20080223-p9erf1mhepf5gc7jy9kek9rgdn.png"/>
+After pressing the *Pull Request* button, you are presented with a preview page
+where you can enter a title and optional description, see exactly what
+commits will be included when the pull request is sent, and also see who the
+pull request will be sent to:
-mojombo can reply to my message, delete it, or ignore it. Pulling from me is
-easy. mojombo can add the defunkt repository as a remote source and fetch its
-contents like this:
+![](http://img.skitch.com/20100831-qit9sjhuqk42t4ww91ifm5tm81.png)
-<pre class="console">
-<span>$</span> git remote add defunkt git://github.com/defunkt/grit.git
-<span>$</span> git fetch defunkt
-</pre>
+If you're sending from a topic branch, the title is pre-filled based on the name
+of the branch. Markdown is supported in the description, so you can embed images
+or use preformatted text blocks.
-The 'git remote add' will stay in your repository configuration, so the next
-time you get a pull request, you only need to fetch again. Note that to fetch
-from a private repository, you need to use the private update URL. If grit were
-private for example, the remote add would look like,
-"git@github.com:defunkt/grit.git".
+Switch to the *Commits* tab to ensure that the correct set of changes is being
+sent:
-N.B. fetch from a remote by default (without -n flag) would also cause all tags
-from that remote to propagate to your local clone, which you might not like.
-Thus you might prefer to use -n or just direct pulls from the remote repository
-with git pull git://github.com/defunkt/grit.git master .
+![](http://img.skitch.com/20100831-c9g4pcsjwnfj14csrpytenxyfn.png)
-Once you have the remote changes fetched locally, you can refer to them as
-'remotename/branchname' to any git tool that accepts revision arguments or
-lists; see the man page for 'git-rev-parse' and/or 'git-rev-list' for more, but
-for example, to see the changes between your 'master' and defunkt's 'master',
- you could use:
+Review the diff of all changes by switching to the *Files Changed* tab:
-<pre class="console">
-<span>$</span> gitk master...defunkt/master
-</pre>
+![](http://img.skitch.com/20100831-qpc5bu8grycefnnbaagnuwbckq.png)
-To see the changes that the branch would introduce as a single patch is slightly
-more complicated; if your two branches have diverged, then a simple 'git diff
-defunkt/master' would show you those changes backed out as well. This is not
-what you want, you wanted to see what the effect of the merge would be, right?
-You could use:
+## Changing The Commit Range and Destination Repository
-<pre class="console">
-<span>$</span> git checkout master
-<span>$</span> git diff $(git merge-base master defunkt/master) defunkt/master
-</pre>
+By default, pull requests are assumed to be based on the parent-most
+repository's integration branch. In this case, the `kneath/jobs` repository was
+forked from `github/jobs` so the pull request is assumed to be based on the
+`master` branch of the `github/jobs` repository. In a great majority of cases,
+the defaults will be right; however, if any of this information is incorrect, press
+the *Change Commits* button.
-See below for a simpler way, if you are a n00b and shell scares you.
+![](http://img.skitch.com/20100831-nm1mgb6n8ng7e4nrdesucqx98h.png)
-To merge the changes done in the defunkt repository to a local branch, e.g., the
-master branch, you can run
+The commit range selector will expand, allowing the base repository, base
+branch, and head branch to be customized:
-<pre class="console">
-<span>$</span> git checkout master
-<span>$</span> git merge defunkt/master
-</pre>
+![](http://img.skitch.com/20100831-pwsq1inmr7m7y61dfcyxnhjkkd.png)
+
+The easiest way of thinking about the commit range is this: the *base branch* is
+**where** you think changes should be applied, the *head branch* is **what** you
+would like to be applied.
+
+Changing the base repository changes who is notified of the pull request.
+Everyone that can push to the base repository will receive an email notification
+and see the new pull request in their dashboard the next time they log in.
+
+Once you're happy with the commit range, press the *Update Commit Range* button
+to update the commit and files changed preview areas.
+
+## Sending The Pull Request
+
+Once you've entered the title and description, made any necessary customizations
+to the commit range, and reviewed the commits and file changes to be sent, press
+the *Send pull request* button.
+
+![](http://img.skitch.com/20100831-fe6i533swbgsgypgc645f999i.png)
+
+The pull request is sent immediately. You're taken to the main pull request
+discussion and review page. Additionally, all repository collaborators and
+followers will see an event in their dashboard:
+
+![](http://img.skitch.com/20100831-bj3rg8bac4buemstnwqy7wwxq2.png)
+
+## Managing Pull Requests
+
+All pull requests sent or received by you are browseable through the pull
+request dashboard.
+
+![](http://img.skitch.com/20100831-xfscxin81wj5j3gwyas2sd398q.png)
-If you are not sure that you want to merge, and the above steps didn't work or
-were too hard, you could use merge with --no-commit and then use git diff;
+Pull requests for a specific repository are also browseable by anyone with
+access by visiting the *Network -> Pull Requests* page.
+
+![](http://img.skitch.com/20100823-bahbpwpemx3jh2kpke77d2dxtc.png)
+
+The pull request dashboard and the repository pull request list support a wide
+range of filtering and sorting controls. Use them to narrow down the list to
+the pull requests you're interested in.
+
+## Reviewing Proposed Changes
+
+When you receive a pull request, the first thing to do is review the set of
+proposed changes. Pull requests are tightly integrated with the underlying git
+repository, so you can see exactly what commits would be merged should the
+request be accepted:
+
+![](http://img.skitch.com/20100831-81im4n771y5tcwg3ryixajtfeg.png)
+
+You can also review the cumulative diff of all file changes across all commits.
+
+![](http://img.skitch.com/20100831-enh8t415ryr5b1sw45shq9ier3.png)
+
+## Pull Request Discussion
+
+After reviewing the basic description, commits, and cumulative diff, the person
+tasked with applying the changes may have questions or comments. Perhaps the
+coding style doesn't match project guideline, or the change is missing unit
+tests, or maybe everything looks great and some props are in order. The
+discussion view is designed to encourage and capture this type of discussion.
+
+![](http://img.skitch.com/20100831-j7fapxihs2a3i48ai5qmqceskp.png)
+
+The discussion view starts with the pull request's original title and
+description and then captures additional activity to display chronologically
+from there. Any of the following types of activity are captured as they happen:
+
+ * Comments left on the pull request itself.
+ * Additional commits pushed to the pull request's branch.
+ * File and line notes left on any of the commits included in the pull request's range.
+
+Pull request comments are Markdown compatibility, so you can embed images, use
+preformatted text blocks, and other formatting supported by Markdown.
+
+## Merging a Pull Request
+
+Once the pull request is deemed satisfactory, someone with push access to the
+destination repository must apply the changes and push the updated branch.
+There are a variety of ways to accomplish this. Two popular methods are
+described below.
+
+#### Fetch and Merge
+
+This is the most common method of fetching and applying changes. It requires
+adding a remote for the person that sent the pull request, fetching from that
+repository, merging the requested branch, fixing any conflicts, and pushing
+the newly merged branch back to the repository:
<pre class="console">
<span>$</span> git checkout master
-<span>$</span> git merge --no-commit defunkt/master
-<span>$</span> git diff HEAD
+<span>$</span> git remote add kneath git://github.com/kneath/jobs.git
+<span>$</span> git fetch kneath
+<span>$</span> git merge kneath/error-page
+<span>$</span> git push origin master
</pre>
-Once you are finished, use 'git commit' to make the merge commit.
+#### Patch and Apply
+
+The *fetch and merge* approach works great when you're working on a team or
+repeatedly applying changes from the same small group of people. Another
+approach that's a bit quicker in one-off cases is to use `git-am`.
-You can also use the git pull helper to do the fetch+merge in one step. This
-comes in handy for one-shot pulls where you do not want to use the same
-repository again in the near future:
+Every pull request has a `.patch` URL where you can grab a textual patch file
+to feed into the `git-am` command:
<pre class="console">
<span>$</span> git checkout master
-<span>$</span> git pull defunkt master
+<span>$</span> curl http://github.com/github/jobs/pull/25.patch | git am
+<span>$</span> git push origin master
</pre>
-Now that you have merged defunkt/grit's master branch, you should probably push
-your branch to let others see the merge.
+## Closing a Pull Request
+
+Pull Requests are automatically closed when the requested commits are merged
+into the destination repository. An event is generated to let all repository
+collaborators and followers know that the merge occurred:
+
+![](http://img.skitch.com/20100831-x9ush35wkyxwmw4sw8yd5sx1eg.png)
+
+It's also possible to manually close a pull request in cases where the set of
+changes are rejected. This is also sometimes necessary if the changes are
+applied with `git-cherry-pick` or using some other mechanism that disallows the
+merge from being detected.