Code review is an essential practice for how we develop software. Not only does it help us ship higher quality code, it's an organic means of knowledge transfer.
One benefit we have of working with a wide variety of teams is that we're able to routinely revisit our practices, question them, and improve our own understanding of them. Sometimes this leads to foregoing the practice all together, but in the case of code reviews we've found time and time again that we shouldn't abandon but embrace it.
This post includes 9 tips for making your code reviews better via pull requests. It's specifically written for the creator of the pull request (aka the person requesting the code review).
1. Remember You're Starting a Conversation
At its core a pull request is nothing more than an enriched conversation. By opening a pull request you're inviting others to take part in a discussion that centers around your work.
These conversations lead to shipping better work, sharing knowledge, teaching, and learning. When kept friendly in tone and thoughtful in exchange they will elevate the entire team.
2. Embrace Vulnerability, i.e. Put Yourself Out There
Putting our work out there exposes us and our thought processes by revealing what we're capable of and what we're not capable of. We become vulnerable through an open invitation for critique.
The upside of putting our work out there is that we give ourselves an opportunity to grow and we provide an opportunity for the team to grow together.
3. Avoid Sloppy Pulls
There's nothing more exigent than the feeling that comes after you finish the last commit on a changeset: Open the pull request, merge the code, move onto the next thing – quickly.
Feeling rushed often leads to a sloppy pull request: Commits aren't cleaned up, the title is vague or misleading, the description doesn't include information necessary for a reviewer, and so on. No one wants to suffer through reviewing a sloppy pull request.
If you find yourself feeling rushed remind yourself to slow down. A short walk away from the computer is a very effective technique.
4. Communicate The "Why?"
The single most helpful piece of information for a reviewer is the answer to this question: Why?
A reviewer who knows the purpose of the pull request can do a better job of reviewing the changeset based on what its intended goal is. This speeds up the review, allows for better feedback, and leads to merging code much quicker.
We usually put the purpose as the very first thing in the pull request's description, if the title isn't clear enough. Wherever it is make sure it's apparent for the reviewer.
Here's one recent pull request:
Pivotal Card: #1234567 (linked to the actual Pivotal Cardd)
This makes it possible to administer API clients from the admin interface.
5. Tell The Reviewer What To Expect
After communicating the why it can be helpful to tell the reviewer what to expect in this changeset. For example:
Pivotal Card: #1234567 (linked to the actual Pivotal Card)
This makes it possible to administer API clients from the admin interface. It requires the logged in user to be an administrator.
- checkout this branch
- sign into the application as an admin
- go to the /admin
- you'll see an "API" navigation item, tap this, then tap on "Clients"
- create, update, and delete to your heart's content
Most of the time text communicates just fine, but don't hold back if a screenshot or short screencast will do a better job.
6. Tell The Reviewer How to Reproduce
When fixing bugs it's helpful for the reviewer to know how to reproduce the bug so they can verify the fix.
If the steps to reproduce already exist in a bug/issue tracking system then you have the option to provide a link in the pull request rather than re-listing all of the steps.
Either way, the reviewer knows what the bug was and will do a better (and quicker) job of confirming it is indeed fixed.
7. Anticipate Feedback
As the pull request creator you know the ins and outs, the whys and hows, and the reasons behind the possible choices you did or didn't make. The challenge for the reviewer is that much of this thinking isn't apparent in the change itself.
Taking a moment to anticipate questions a reviewer may have often helps streamline the review by reducing the back and forth.
For us, this not only leads to more informative pull requests, but we also go back and clean up commits and their messages making them more informative.
8. Respond to Comments That Need It
Once feedback starts rolling in be sure to respond to comments that desire a response. Doing so helps everyone keep track of the feedback that has been seen and considered.
These comments may be direct questions:
Should this method live on Person instead of Company?
Or they may be indirect questions:
Not sure about using Nokogiri here.
Or possibly even suggestions:
Perhaps it would be best to denote an authentication scheme to be used. e.g., Authorization: Token AN_API_TOKEN
Responding doesn't mean you have to abide by each piece of feedback, but it does acknowledge reviewer's time and effort reviewing your work.
A helpful technique that we've found is doing a mini review before we open a pull request. A few of the things we look for in our work:
- Are the commits in good order, clean, and tidy?
- Are there extraneous things in any commits that should be split into their own?
- Are the commit messages clear and informative?
- Should anything be opened in its own pull request?
- Are there any missing tests?
By putting on the reviewer hat we often find things that we may have overlooked while creating the changes.
Some pull requests are so small, clear, and tidy that they take all but a minute or two to get merged. Others may be larger with loftier goals or creative solutions to complex problems. Of course, there's quite the variety in between.
The discussions that take place have provided some of the most insightful questions, suggestions, or solutions that we've seen.
Opening every pull request with the goal of getting the most out of both the work and the review itself has made peer review an engaging activity amongst peers and colleagues, at least for me.
We hope these tips helps you find your rhythm for framing and taking part in these conversations.