Feature Branches and Pull Requests : Walkthrough

Now that a Pull Request is live, someone has to review it. Let's walk through some of the steps involved in making this process go quickly.

Finding Pull Requests

First we need to get to the PR. From a project's main page, we can see that Pull Requests are a menu option.

Click that to see a list of all the open PR's.

For this example, I'm going to focus on my Generated Docs PR .

Reviewing a PR

As a reviewer, its my job to check out the PR for any major issues, as well as comment on smaller issues I find in the code. I think a goal here is to be through but balance your time and the developer's time.

On a PR's main page, I can see the PR's description and any existing comments about the code.

Start here to understand the scope of the request.

Then, skip to the File Changes tab to start reviewing.

(So far, I haven't found the Commits tab all that useful for reviewing the code - it gets too fragmented when there are multiple changes).

Commenting on Issues

So the PR isn't perfect. No biggie - that is expected. The review process makes it easy for a reviewer to spot issues and for the coder to implement changes.

As a reviewer, you can add comments inline to the change preview on github. Simply click on the little + next to the line or lines you have issue with.

As the PR requester, you can comment on those comments to justify or suggest future actions.

Here is an example where Yannick catches a spot where I left in some old code.

I follow up, indicating that it will be dealt with on this branch

Dealing with Issues

Now that there is a problem with the PR, we need to fix it. The PR process makes this pretty easy too.

Locally, on my feature branch, I simply implement the changes to fix the problem and commit the changes.

Then I push those changes up to the remote branch again. In this case, the branch was called docs. So:

git push origin docs

The PR is automatically updated with the latest changes. As Yannicks comments are on code affected by this new commit, they are hidden by default.

Merging PR

After a few messages, we should arrive at a desired +1 - which means its time to get this merged.

Additional suggestions can be dealt with in a separate PR - or added as another commit to the existing one - if you feel the scope is small enough. I typically like smaller PRs to bigger ones, so if I can justify splitting out additional suggestions into separate issues, I go that route.

If you are lucky, the PR can be automerged - and you can do it right inside github!

But sometimes (and not often honestly), the magic doesn't quite work, and you need to return to the command line one final time for the merge.

Luckily, the instructions for getting started are right there, just a click away.

First make sure you have the latest master in your local repo.

git checkout master
git pull origin master

Then switch back to the feature branch and merge in master

git checkout docs
git merge master

This should fail, and it should tell you which files you need to look at to resolve the conflict.

Here, I just have one file that needs attention gruntfile.js

Opening it up in my text editor, i look for the <<<< indicating a merge conflict.

I modify the code until all the conflicts are dealt with. Then add the file to staging.

git add gruntfile.js

The rest of the merge is already in staging - so now I can commit the merge

git commit -m 'merged master'

And switch back to master to push up this change

git checkout master
git merge --no-ff docs
git push origin master

A lot more work then the auto-merge -but hopefully its unusual that auto-merge isn't an option

Delete remote branch

Either way, once the PR is merged into master, we can remove the remote branch - which keeps our github project cleaned up.