As someone who is currently reviewing all Pull Requests for a project, I’ve been working through ways to become more effective and efficient at this process.
Code review as part of a Git workflow is not a new thing. Scott Chaccon’s post is more than three years old at this point, but the original idea he expresses in the post is still very valid.
At Sparkbox, we use the Github flow model for code review. What this means is that anything in master is production-ready and deployable. Any ongoing development work is done in feature branches and later merged into master. As fellow developers travel or work from home, the need for a system of code review that doesn’t rely on physically meeting and walking through code with another person becomes more important. Pull Requests enable code review from anywhere at any time, which is why we’ve switched to using Pull Requests for all code review.
Pull Requests
Pull Requests (PRs) give us a thread for a set of commits, be they new features or bug fixes. This thread is important because it logs the evolution of a feature or fix. The PR thread also demonstrates how valuable the input of other developers is to a successful project.
PRs increase collaboration, provide guidance, and keep everyone on the team feeling supported. That last part might be the most important. We try to avoid isolating a single developer on a project, but sometimes a developer will be the only one assigned to a project. That’s why we’ve decided that even if someone is the only full-time developer on the project, it’s important for another developer to be available to review PRs. This keeps the quality of the code we produce high, and it keeps everyone on the team feeling like they are on a team, not flying solo.
Managing Lots of Pull Requests
If you’re someone who is reviewing many PRs a day, it can be hard to keep up with the hoard of them. I’ve found some of these things help wrangle the mess.
Set a specific time to check PRs. This keeps your PRs from taking away all your time (in the same way as email), and lets you still accomplish your own development tasks.
Don’t just use the Github generated diff; pull the branch down and test it yourself.
Don’t be afraid to add additional edits and commits if you think it will save time on back-and-forth communication. This is a hard thing to balance, but learning to balance fixing a small thing versus kicking the code back to the developer will greatly decrease the time a PR is stuck in “review mode.”
Tools to Help
Here are some of the tools I find essential in reviewing PRs and working in git in general.
I love the Vim Fugitive plugin. It lets me effortlessly see git blame data, generates a great diff view, and lets me get the status of my branch without jumping back to the command line. To get started with Vim Fugitive, I highly recommend the vimcasts episodes on Fugitive.
I’ve recently been experimenting with hub, and its faster sibling gh. Both of these offer some nice sugar on top of git’s already verbose command tree.
Jeremy Mack in our office recently started experimenting with Bee for managing issues and PRs.
Neil has mentioned it before, but I’ll put another plug in for it: gitsh is a fantastic addition to any command line setup. The bonus part is that if you’re using hub or gh, you can specify gitsh to use those instead of standard git, by loading up gitsh like this “gitsh --git gh.”
Merging PRs
There are also a few approaches to merging in PRs once the code has been reviewed and approved.
You can just use the green, merge-pull-request button on the Github website, and it’s generally fine. There has been some blow back in our office about how Github fails to “fast-forward” merged PRs, though. This can cause the history of a project to look a little strange.
You can also merge directly in the command line to avoid the issues of the merge button on Github.
Another approach I’ve been experimenting with uses hub and git am.
Collaboration Requires the Right People
It takes a secure person and team to effectively use a PR model for code review, but the benefits are pretty great—especially when managing remote and local developers on the same project. We’re only as good as our collaboration.
