5 Actionable Tips To Deliver Higher Quality Code Reviews Today

Reviewing is a core process in software development. It is a form of communication between two parties with different purposes. As with any form of communication, they can always be improved upon and the reviewing process is not an exception. Both the code proposer and the code reviewer play their parts, but in this article, we’ll focus on the reviewer.

As a reviewer, you have a lot of different responsibilities. First of all, you’re one of the first rounds of quality assessment for the Merge Request (MR). Secondly, you’re basically representing the team to make sure that the proposed code conforms to the standards of the team. Thirdly, you’re expected to dive into the code and point out potential errors, whether minor details or overarching code structures. Lastly, you have to actively communicate all your feedback and potentially follow up on any discussion that can arise from it.

Because there are so many responsibilities, it’s easy to get caught up in our own perspectives as the reviewer and forget about the person on the other side. The problem is that every form of communication is a two-sided thing. Your colleagues will have to process whatever comes out of your review.

In the end, this entire process is a team effort. In that context, optimising their end of the workflow is also part of your job as a colleague. But how do you become a better reviewer and improve the quality of your code reviews?

This article will cover 5 concrete improvements you can do to improve the quality of your reviews. In turn, this will help you become a better reviewer, provide more meaningful reviews to your colleagues, prevent unnecessary discussions, and in general optimize the review process inside the team.

Provide Suggestions For Solutions

Contrary to popular beliefs, the purpose of reviewing your colleagues’ code is not solely to spot and highlight errors. Most of the time, a developer will create an MR when they think it is finished. If this is not already the case yet, then it definitely should be. Otherwise, they are basically wasting the time of their colleagues who are reviewing code that might be unfinished.

But under the assumption that the code of the MR is finished, solely pointing out errors is often not too meaningful for your colleague. The code that you are looking at is what your colleague has settled on. Unless the errors are very minor or small, there are most likely reasons that they have decided to approach it as they did.

Solely pointing out the errors will only make the code proposer more confused. If they knew how to implement it in a better way, then they would’ve done so already. Instead, they now only know that it’s wrong, but still don’t necessarily know how to address it properly. In the end, this will only create more back-and-forth discussions between them and you, which will consume more time and effort from the both of you.

Instead, providing suggestions on how to address the issue you’re pointing out will drastically change this process. Not only will the other developer know exactly what you thought was wrong about their implementation, but your intentions are also immediately clear. You are the one that is in the best position to explain those intentions. Since you are the one pointing it out, you will also have the best idea of how to improve it.

Providing suggestions for solutions in reviews can do a lot for both parties. The other party doesn’t have to go through several discussions with you to clarify your intentions. Either it’s immediately clear for them what to do or they can immediately respond to your suggestion with targeted questions. In the end, this is a way more efficient and effective usage of time for both them and you.

Normal reviews only tell the developer what is wrong, better reviews think along and make suggestions.

Be Prepared And Committed To Following Up

Nothing feels worse than not gaining a response after committing to a review, which goes both ways. Your colleagues have spent valuable time understanding the requirements, coming up with a solution, and implementing everything. Afterwards, they also have to spend time going through your feedback, understanding it, and processing it.

The worse feeling for them would be that someone comes by, only points out a bunch of errors in their MR, provides them more work with unclear instructions, and then disappears. Not only do they need you for the review, but also for everything related to the outcome of your review: discussing, understanding, processing, and ultimately verifying the feedback.

The other way around, when you’re the proposer and they’re reviewing your work, you wouldn’t want that to happen either. As a reviewer, make sure you’re prepared and committed to following up. Whether it’s a response to your feedback, discussion, or another review, they’re all additional responsibilities for a reviewer. Even small gestures like an explicit thumbs-up can make a world of difference. Being prepared and committed to those will tremendously help in providing better reviews and becoming a better reviewer.

Make Your Intentions Clear

Through textual communication, it’s extremely hard to understand context and sentiment. It’s a very difficult process, therefore it’s thus more important that you’re as clear and explicit as possible. Oftentimes, we believe that we’re very clear in our messages, comments, and feedback. In reality, this is rarely the case. Personally, I have experienced from both sides of a discussion that feedback wasn’t clear.

But in hindsight, it’s relatively easy to recognise whether your intentions were properly communicated and received or not. Imagine scenarios where you started with some feedback, but the intentions of either the feedback or the response to the feedback aren’t entirely clear. Most of the time, people will try to solve it asynchronously through comments. But in the end, this still doesn’t work and you decide that it’s easier to jump into a Zoom call and talk it out in a synchronous way.

While for the situation it’s a perfectly fine solution to avoid even more wasted time, it’s indicative that the previous communication wasn’t clear enough and that an amount of time has already been wasted. Because if it was clear and explicit enough before that, a Zoom meeting wouldn’t have been necessary in the first place.

That’s why you should always try to be as clear and explicit as possible, especially if you have specific intentions in mind with your feedback or comments. Being critical and pointing out mistakes are part of the reviewing process, and I’m not discouraging you from doing so. But don’t immediately assume that your feedback is clear enough for the code proposer to follow up upon.

Double-check your comments and ask yourself whether you would be able to do what you want them to do based on solely your comments. If not, add the remaining pieces. Be extremely specific and explicit about your reasonings, intentions, and expectations. With that, you’re helping the code proposer way more than you think and will avoid wasted time and effort for both parties.

Make Use Of Nits

When reviewing someone’s code, there are a lot of issues that you can encounter. Some are more relevant or important to mention than others. In certain cases, your reasons for rejecting an MR are only represented by a small subset of comments in your review, which means that the other comments are less important. In other cases, you just want to highlight something without expectations that the proposer will immediately address it.

A very easy solution to clearly communicate the (lack of) importance of these comments is by making use of nits. Marking something as a nit means you see it as a small imperfection in the code. It could be improved upon but there’s no immediate need to do so. Either it’s not a high priority, it’s personal preference, or it doesn’t affect the code significantly.

The easiest way that I use to mark something as a nit is to start your comment with “Nit:” and then describe your feedback. This is not the only way and there are a lot of different ways to describe the above intentions explicitly. Regardless of your method, the most important thing is to make it clear that you don’t expect the proposer to necessarily address it.

You have left the choice with the code proposer. If they think it’s worthwhile or relevant to include your nit feedback, then they are allowed to do so. If they think it’s not worthwhile enough or too much effort to include it in the current MR, then that’s fine too. You’re making it clear to them that they can make the final decision without the need for another back-and-forth with you.

And this was all made possible by the minimal effort of adding just one word to your comments.

Distinguish Between Necessary and Preference

As a reviewer, it’s very important to keep in mind what the intentions behind your feedback are. Ultimately, the purpose of reviewing is to ensure that all the code that gets into the codebase is of a certain level of quality that matches the team’s standards. If there are significant or clear shortcomings in the quality of the proposed code, then it’s necessary to address that during a review. Think of major topics like the overall code structure, architecture, and performance.

What all of these have in common is that their intention is ensuring that the code meets the quality standards of the team. This intention is what makes the feedback of high priority and importance.

It’s not uncommon for a review to also include feedback that is very specifically targeting the chosen implementation or details of it. Most of the time, this feedback originates from the reviewer disagreeing with the MR’s specific implementation. The issue with this type of feedback is that they more often than not don’t provide any meaningful value to the review.

Speaking from a personal experience, I’m a huge fan of functional array methods in JavaScript like Array.reduce, Array.filter, and Array.map. If possible, I would use these all the time and preferred them over alternative implementations.

In the past, I was overly allergic to coming across these alternatives in code review. Every time I encountered a for-loop in someone else’s code, I would immediately include a comment about it in my review. The main reason being that I thought that code construction was ugly, verbose, and generally just not how I preferred to see it.

But looking at it in hindsight, the only thing that this “enforcement of preferences” did was spark unnecessary conversations between my colleagues and me. Discussing whether either implementation had a significant advantage, whether it was really necessary, whether it made any difference, or what my expectations were. In the grand scheme of things, all of these discussions weren’t necessary and could’ve been avoided if I didn’t push my own personal preference for code onto the code proposer in the review.

As a code reviewer, there are a lot of topics that you can pay attention to. Obviously, it would be great if your review would cover all of them, but ultimately it’s most important that the feedback is meaningful. You should consider whether the feedback is necessary, and thus holds meaningful value, or whether it’s more personal preference, and thus holds less value or will even hinder the MR.

Review like it’s your code, but accept that it’s not your code.

Final Thoughts

Being able to deliver good reviews is not only beneficial for the code proposer, but also the code, the team, and ultimately yourself. A lot of time in the review process is wasted on figuring out the other party’s intentions, waiting, and discussions. The most optimal solution would be to avoid all of those confusions. The first step towards that goal is becoming a better reviewer yourself and delivering higher-quality reviews.

To help you with that, this article covered 5 concrete tips. To start, it’s recommended to provide suggestions for solutions instead of just pointing out errors. This is way more meaningful for the proposer and will save you a lot of discussions and time. Talking about time, make sure that you’re prepared and committed to following up on feedback from the code proposer to keep the ball rolling. Nobody likes their work to be stalled for two weeks.

It also doesn’t hurt to make your intentions even more explicitly clear. A reviewing process is a form of communication and the golden rule with communication is: the clearer the better. To communicate even better, learn to make use of nits so the proposer knows whether specific comments are a priority or not. Lastly, remember to distinguish whether your feedback is really necessary or just personal preference.