by Jake Worth on October 8, 2015

The dreaded code review. Nobody likes receiving or delivering a code review, but scrutinizing our code leads to a better product.

"In my reviews, I feel it's good to make it clear that I'm not proposing objective truth, but subjective reactions; a review should reflect the immediate experience." – Roger Ebert

Code reviews are crucial – one set of eyes on a codebase is not enough. Modern applications are simply too complex for one person to manage alone. We review code to prevent regressions, document behavior, ensure quality, and share knowledge. The ability to justify and explain your design choices is an essential skill for any developer.

(I'm using the term 'code review' here in the same way others might say 'code audit': I'm reviewing a pull request on a project I maintain from a developer I don't yet have a relationship with. I'm the gatekeeper, and and my goal is to approve the changes or suggest improvements.)

In this post, I'd like to discuss the way I approach a code review. Everybody does this their own way, but I have a checklist, some tools, and general principles that help me as I review.

The first thing I ask is: do the existing tests pass? This is a no-brainer. The new feature can't introduce regressions or break tests.

Next, I look for tests that document the new behavior. We keep our code test-driven at Hashrocket, and expect to see tests for any feature. Having tests goes a long way toward preventing regressions and conveying intention. If a model is introduced, I look for unit tests. If it's a feature, I look for integration tests.

My next question is, does it work? Fire up your browser and walk through the story. If you can't manually test the feature in a few minutes, the story (or documentation) is unclear.

I like to look at the commits, too. Good commits should be atomic: the tests should pass at every step, and the code should be moving toward a realized feature. The commits should be concise and intention-revealing. Anything vague during a review is going to be unintelligible in the future, because the author won't be there to explain. A good git rebase can make anybody look like a genius.

Here is an example:

commit 55jc0bc8e5f1ff4979699b400001f603011e2256
Author: Developer Alpha and Developer Bravo <dev+deva+devb@hashrocket.com>
Date:   Tue Sep 29 09:50:25 2015 -0500

    Add middleman instructions to README

    diff --git a/README.md b/README.md
    index f02eeb3..93919a2 100644
    --- a/README.md
    +++ b/README.md

This message explains exactly what the commit does, in the tense (present) we tend to use on our projects. At 35 characters, it's short enough that no content will be wrapped by any view on Github. It states that it only changed one file, and then only changes one file. If you checkout this commit in Git and run the tests, they pass. Tim Pope wrote about this in 2008, and the advice is still relevant today.

Other factors to consider: does the code use design patterns already established in the project? Does it conform to company and technology-specific styles?

There are many tools to help facilitate a review. Github comments are useful for talking about specific lines of code. Code linters like Rubocop, HAML-lint, or Hound can be used to find style violations. There are static analyzers for code smells like Code Climate, security like Brakeman, redundant queries like Bullet, coverage like SimpleCov. Like Sandi Metz's ideas? There's a gem (SandiMeter) for that.

Some of these can even be configured with Github to automatically check against any pull request. Make the computers do the work!

Be nice. It's easy to shoot down somebody's idea, but it's hard to build on an idea or provide a thoughtful alternative. Everybody is trying to write the best code they can. If you are receiving a code review, remember that you are not your code.

Teach. Code reviews can be a great opportunity to teach. Some of my favorite techniques have come from people reviewing my code. Don't miss an opportunity to explain.

Pair. Pair programming is like a continual, in-person code review, and it beats a traditional review because it's happening as you type.

Code reviews matter. With these guidelines, tools, and techniques, I can critique anyone's code productively. Better code helps our clients, company, and the open source community continue to thrive.