Building Code Review Speed — Guidelines for Fast & Effective CR

Code Review (CR) is the methodology of changing the codebase while keeping the code healthy.

At one end of this process, we have the developer whose job is to complete a code change while meeting deadlines. On the other side, we have the reviewer whose job is protecting the codebase from degradation.

It’s in the best interest of all that CR will be fast without compromising quality. In this post, I’ll describe some guidelines for achieving this goal.

Code Health

The overall quality of a codebase is called “Code health.” A healthy code is:

Teams with healthy code have high code velocity, and low frustration levels (of the team, clients, and users).

A project with low code health is most likely somewhere on the path to failure. It could include:

Types of CRs

There are many methodologies of code review. I find “email pass-around” the most effective. For this method:

This approach has a few advantages:

Building Code Review Speed

For agile teams, CR is one step in the development pipeline.

By slowing down a single step, this can reduce the team’s productivity because it blocks later steps (i.e. our QA team waits for a feature to be completed before the e2e automation test can be finished).

Additionally, slow CR means code changes are pending for a long time, increasing the possibility of code-merge conflicts and QA regressions.

Finally, when time is of the essence and corner-cutting is needed, slow CR is one of the first steps to go out the window.

CR Guidelines

CR is all about compromises. There is no such thing as the “perfect code.”

Developers may need to swallow their pride and make changes to the code they do not agree with.

Reviewers may need to do the same and admit that the developer code is not that bad after all (even though it is well known that the worst code is written by others).

The most important rule of CR is:

Reviewers should approve the code submission when it improves the overall code health.

Obviously, when this moment occurs, it is subjective.

We can make things more objective by adding a few more rules

Avoid Personal Preferences

Some developers like indenting using tabs; other white spaces.

Using a style guide reduces such conflicts. There’s no need to re-invent the wheel here. There are plenty of style guides out there, for example, Google & Airbnb. Adopt one that the team can agree on.

For some things, even a style guide won’t offer you clear answers. In such cases, try to make your case with hard data.

For example, a while back I came across a debate about CR that focused on the best way to verify if a string contains whitespace.

Vs:

Luckily, there are ways to get empirical data and resolve the matter by testing it.

Use CR Tools

When there are well-defined rules on how code should be written, we can enforce them automatically. Static code analyses can reduce CR noise drastically.

Avoid Large Code Changes

Studies show that:

More than that and people will get tired and out of focus. Plus, the CR quality will likely drop.

Avoid sending massive code changes to review. Instead, split the changes into smaller bite-sized chunks.

Avoid Long Discussions

When submitting code for review, make it easy for the reviewer to understand what you’ve done. Think ahead of questions he or she may ask, and add the details to the review request (i.e to the git commit message or pull request).

When reviewing code, do it all in one go, and do it well.

As a general rule, the feedback loop should be short. One or two cycles should cover it. If more cycles are needed, prefer to move discussions into a face-to-face meeting.

Resolve Conflicts ASAP

When the developer and reviewer disagree on a matter, it should be resolved quickly.

What to Look for During CRs:

While reviewing a code change, there are a number of aspects to focus on.

Design

Functionality

Complexity/Readability

Testing

How to be a Good Reviewer

A Good Reviewer is Thorough

A Good Reviewer is Responsive

A Good Reviewer is Kind

A Good Reviewer is Informative

A Good Reviewer is a Mentor

A Good Reviewer Shows Trust

Originally published at https://guyogev.com on August 12, 2019.

Full stack web developer that likes to code.