Frustration-Free Code Reviews

Hopefully, code reviews are already a part of your software development process. They should be a collaborative, non-judgemental tool for improving code quality and decreasing knowledge silos. Yet instead, they often serve as a source of frustration and fear. But why?

Well, if this was a play we’d have the following parts:

• The code
• The Author
• The Author’s Ego
• The Reviewer
• The Reviewer’s Ego

As you can imagine bringing this cast of characters together can be a challenge! Especially since we want to do so in a way that results in high-quality software. Thus we need a plan, just like any good play has a well-written script.

Let’s look at the following areas and how they contribute to a frustration-free code review:

• Goals
• Pull Requests
• CI Integration
• Giving Comments
• Responding to Comments

Goals

First, both the author and the reviewer need to have clear goals for the code review. By knowing what to expect authors are more likely to feel comfortable asking for the review. And reviewers know what they should focus on. I personally look for the following things:

• Readability
• Duplicated logic
• Business logic
• Opportunities for a Design Pattern
• Use of Standard library functions

By having the goal in mind, we can have more focused comments and critique. I love this diagram from Alex Hill, where she outlines the various places a comment could fall during a code review.

For those items in the Low reward/High conflict area, we should have automation set up in our projects to report on those things. I always feel a bit icky pointing out to someone that there’s an extra newline or a missing space after an if statement. With static code analysis tools that’s no longer my job; I can focus on more High reward items.

My primary application is written in Kotlin, so we use the Detekt library to perform analysis of our code. Not only do we get results from the command line, there is also an IDE plugin available to provide feedback real-time.

Pull Requests

Once you as the author feel your code is ready to go, the next step is to open a Pull Request. By default, GitHub will slap the first line of your commit message as the title for the PR and that’s it. But there’s so much more you can do with your PRs to ensure a successful code review.

First, make sure that the title includes the issue number that your code is related to along with some human-understandable summary. Here’s the format I use:

Fixed [#ZS-1234]: Added ability to display past user reports

It covers what I’ve done and what the related issue was. But let’s say I would like a code review and I’m not 100% finished with the ticket. Then I would use this:

Partial [#ZS-1234]: Added ability to load past user reports

I like this approach because the reviewer at a quick glance can get a sense of what the PR is about and any potential time investment they may need to review it.

The next piece of the puzzle is to have a template that automatically populates the description field when a PR is opened. I really like it because it sets clear expectations for the person who opens the PR. It also motivates the reviewer to focus on the primary goals of the review instead of housekeeping tasks.

In order to add one, you would create a file named PULL_REQUEST_TEMPLATE.md and place it in one of three locations:

• Root of your project
• hidden .github folder
• docs folder

Once your commit with the template has been merged into your default branch GitHub will auto-populate new PRs with the contents. Here’s the template I use for my Android applications at work:

Adapted from: https://git.io/fA8D6

You can customize it to suit the needs of your team. Our application is available in several languages, so we have to push any new strings to our translation service for translation. In the past when we’ve missed this step it has caused issues during the release process. Having it as an item in the checklist serves as a constant reminder to the author and saves the reviewer from having to go and check that it was done.

CI Integration

For this next bit, you will need to have a Continuous Integration environment in place. At my current company we use Jenkins, but any modern CI will do the job. We’re going to configure it so that we can take advantage of the results of the Detekt static analysis results directly inside of a Pull Request. We will use the Violation Comments to GitHub plugin. It searches through your /build directory for report files from static code analysis tools and comments PRs in GitHub with them.

Here’s how you can configure it inside of your application’s build.gradle file.

apply plugin: "se.bjurr.violations.violation-comments-to-github-gradle-plugin"

task violationCommentsToGitHub(type: ViolationCommentsToGitHubTask) {
    repositoryOwner = "owner_name"
    repositoryName = "repo_name"
    pullRequestId = System.properties['GITHUB_PULLREQUESTID']
    oAuth2Token = System.properties['GITHUB_OAUTH2TOKEN']
    gitHubUrl = "https://api.github.com/"
    createCommentWithAllSingleFileComments = false
    createSingleFileComments = true
    commentOnlyChangedContent = true
    minSeverity = SEVERITY.INFO
    commentTemplate = """
**Reporter**: {{violation.reporter}}{{#violation.rule}}

**Rule**: {{violation.rule}}{{/violation.rule}}
**Severity**: {{violation.severity}}
**File**: {{violation.file}} L{{violation.startLine}}{{#violation.source}}

**Source**: {{violation.source}}{{/violation.source}}

{{violation.message}}
"""
    violations = [
            ["CHECKSTYLE", ".", ".*/detekt/.*\\.xml\$", "Detekt"]
    ]
}

You need to provide the items indicated in bold above. Then on your CI server after your application has been built, you would run the following command:

./gradlew violationCommentsToGitHub \
-DGITHUB_PULLREQUESTID=<id> -DGITHUB_OAUTH2TOKEN=<token>

Here’s an example of the type of comment that you would receive in your Pull Request:

Now, right inside of GitHub the author has the opportunity to address any reported Low reward/High conflict issues without reviewer intervention.

Giving Comments

Eventually, as a reviewer, you will need to provide comments. How can you do so in a way that is empathetic and sensitive to the feelings, and let’s face it, egos of your fellow developers? Here are a few tips that I keep in mind when leaving comments:

• Include at least one positive comment, “Thanks for the refactor here!”
• Include myself, e.g. “Why did we want to…here?”
• Comment in the form of a question, “Have you been able to…?”
• Focus on the code, not the person, “This line confuses me. Is it…?”

Then if I find myself leaving too many comments. I arrange to have a quick meeting first to talk it over. Then I come back and add follow-up comments. This helps the author understand that they’re not being attacked. It also provides me with additional context that sometimes eliminates the need for as many comments on the PR.

Despite my best efforts sometimes my comments can be perceived as a little harsh. Maybe that is the case for you as well. For those times, I make liberal use of the following emojis which seems to help smooth things over: 🤗🙃🙈. Give it a try!

Responding to Comments

After the reviewer has taken the time to provide you with actionable comments. It’s best practice to acknowledge each comment. Even if it’s a simple thumbs up 👍🏽. This lets the reviewer know that you’ve seen the comment and you have taken note or will address it.

Also, it’s hard to have someone critique your work, but try not to take it personally. The focus of the review is the code and the sharing of knowledge, not you. If a particular comment or question rubs you the wrong way, first assume best intent and then pose a follow-up question to really draw out what the reviewer meant.

We want to have multiple pairs of eyes on our code to help find issues. Code reviews give us this power. Yet with great power, comes great responsibility. So use these tools and techniques to make your next code review a frustration-free one!