Rescuing Code Reviews to Build Great Software and Stronger Teams

Ah, code reviews: that venerable old soldier, commissioned to faithfully protect the quality of our software. Yet, so often code reviews become the battleground of egos and, if we are not careful, a black hole of time. This need not be.

A well done impartial inspection of software helps prevent costly errors, ensures a quality product, and is an investment in the technical strength of the team and long-term health of the software system, which is why they are a standard practice at Bounteous.

Let’s take a look at a handful of tips to ensure your code reviews serve to improve both your software quality and the strength of your team.

Start a Conversation

Code reviews should be the start of a conversation, where reviewer and developer discuss the pull request (PR) at hand, ways it can be improved, why it works the way it does, and any problems with the code. It should not be an assault by the reviewer demanding immediate surrender to her views, or a heroic stand by the developer, defending his code to the death.

Rather, by working together with effective, open communication, the developer and reviewer(s) should form a team with a shared primary goal to improve the quality of the code. They should also have a secondary goal of strengthening each other, sharing insight the other may lack. Remember: you are interacting with people, not just code. The rest of the tips follow from the simple principle. Do that, and you will improve both the code and the team.

Be Specific

Don’t make the developer guess at what the problem is or what you are thinking. Give enough details for them to see the issue the same way you do. Comments like “this is incorrect,” or “this needs fixing,” will be the start of a conversation, but not one that needs to take place. Save comment cycles and be specific the first time.

Be Open-minded

Assume the developer knows more about the code under review than you do. Give them the benefit of the doubt and ask for an explanation for something that seems off. Your solution may not be the only correct way to solve the problem at hand, and it is better for everyone if you ask, instead of wrongly assuming that only you could be right.

Share The Wealth

If you have insight into the code, or the context of the code, that the developer does not, politely educate them. This is especially important when working with less experienced developers, or those new to the project. Telling them what to change without explaining why is just “giving them a fish,” instead of “teaching them how to fish,” as the old adage goes. Redeem the time by improving both the code and your team member.

Be Polite

When using technology to comment on technology it can be easy to forget that there is a person on the other end receiving your comment. This person may have spent a fair bit of time crafting the code you are reviewing and may have a personal vested interest in it. At least we hope so. Choose a tone of voice that honors the person and effort. For some people, it can be stressful to be under the spotlight. A polite tone can reduce tension and prevent the process from devolving into a defensive battle.

With any list of tips, there are plenty of counter-examples, ways to do it wrong. Let me draw from my own experiences for “what not to do.”

Don’t Be NSFW

A manager once provided feedback on a PR telling the developer it was “#$!@.“ That was not specific, polite or helpful, and I doubt it did much for the developer's relationship with the individual. And it doesn’t help improve the code either. Don’t be like that guy. Keep it professional. Remember: PR comments live forever; make sure you really want to be remembered that way.

Don’t Abuse The Power

In most processes, code cannot be merged or promoted until it has been approved by the reviewer(s). Don’t let that power go to your head. A reviewer once insisted that a certain library should be used based on his textbook understanding of Java performance. He did not understand compiler optimization or the proper use of that library. After almost two days of cajoling, explaining, and, finally showing him the decompiled bytecode, we convinced him that the change he demanded was not appropriate. This extreme example of stubbornness illustrates how much time can be wasted when we dig in our heels and cling to the power of approval too tightly.

Avoid Defect Envy

I am afraid I am the culprit in this story. As a young developer, I chafed with embarrassment at the number of comments I received during a code review. Out to prove I had the chops to keep up with the more experienced developers, and possibly motivated a little by revenge, I made all sorts of petty comments at the next code review. I was accused of having “defect envy.” Busted. Comments should be helpful and necessary, not just a demonstration of your observational prowess.

Be Open-minded

Wait, didn’t we cover that already? Yes, as the reviewer. But the developer also needs to be open to constructive feedback. I recall once a developer who refused design feedback from multiple subject matter experts. His solution “worked” but required significant redesigns and awkward implementations to accommodate the less-than-optimal design. Baking in technical debt from the outset could have been avoided with a more receptive mindset.

Don’t Weaponize

While kibitzing with a former coworker about this article, he recalled a time when personal conflict found less-than-friendly expression in his code review, further straining already fragile relationships. This team did not have the best interpersonal dynamics. Code reviews are a relational act, and should not be used against others. Build bridges, not arsenals.

Wrapping It Up

There are plenty of other simple things we can do to rescue our code reviews from conflict, time drains and other counter-productive tendencies. Treat it as a conversation with a good friend and let common sense take it from there. It will go a long way to building great software and even better teams.