For many software companies, code reviews are an essential part of the project lifecycle. Code reviews allow us to have multiple sets of eyes over a piece of code before testing and deployment. Quite often, the reviewers will raise feedback to the author, noting areas of the newly added or changed code which could be improved, or potentially even flagging issues before the code is ever run.
Effective code reviews require striking a careful balance between technology and communication. Proper consideration needs to be given to the code, and any quality or functionality issues. Identifying issues is all well and good, but if you cannot deliver the feedback effectively and respectfully, it might do more harm than good to your team cohesion. The point of a code review is not to show that you are smarter than the other person, or to point out everything they did wrong. At its core, a code review is a collaborative process, and a quality filter to ensure that your team’s outputs are as good as they can be. After all, no one produces perfect work, no matter how much experience they have.
How, then, can we structure our code reviews to ensure that people are respected, and feedback is heard? The following is a set of soft principles which I have found to be effective when giving and receiving code review feedback. These aren’t silver bullets, and your mileage may vary.
Us against the problem
A principle which is more often applied to relationship advice, using this to frame all of your feedback is very effective. Your feedback should not target the author personally; it should target the issue in the code itself. Rather than writing “You did this wrong”, flip it around and focus on the problem itself. Of course, even if you do flip it around, it isn’t constructive to say “This is wrong”, which leads us to our next point.
Offer a solution, not just a problem
“This is wrong” is pretty much the epitome of unhelpful. Of course, in some cases, it might be something simple like a typo, but sometimes you will encounter implementation issues where the approach in question will not work. In these cases, it is important to offer a potential solution to the problem. After all, if the author knew the “correct” way of doing it, chances are if it is a deeper issue than a typo, they probably wouldn’t have included the issue in the first place. For this reason, rather than “This is wrong”, it is preferable to take the time to provide extra detail around why you believe it is an issue, and how it could potentially be solved. A potential piece of feedback following this principle might look something like “This will not work because of {reason}. Instead, consider {alternative}”.
Keep pull requests short
Ask any people-manager, giving good feedback is time-consuming. One issue which you have likely noticed if you’ve been involved with a variety of code reviews is that long pull requests don’t tend to get the same level of care and thought as their short counterparts. This isn’t always the case of course, but it is hard to argue against the fact that it is easier to give in-depth feedback on a small diff of say 60 lines than it would be to review one of 600 lines.
Naturally, all 600 of those lines are going to need to be reviewed at some point. By following good ticket and branch management processes, and decomposing work to small enough chunks, you can significantly reduce the cognitive burden of any given code review, which will result in more focus, and high-quality feedback.
Not only criticism
At its core, code reviews are essential feedback sessions. Because of this, you should avoid only ever giving critical feedback. Critical feedback is so vital to personal growth because it highlights what we can improve and do better, but it is only one side of the equation. To be an effective giver of feedback, you also need to offer praise when people do good things, which they should continue doing. To this end, when you see something particularly noteworthy in a code review, it can be a great idea to flag it as being well done.
Do note that the other principles still apply here. Do not praise the author directly. Telling someone “you’re great” is not good praise, because being great is not something which someone can do more of explicitly. Instead, focus on what they did and why it is noteworthy. Flagging someone’s approach to solving a particular problem and saying “I like this solution because {reason}, good work” is vastly more helpful, because it focuses on what the author has done, rather than who or what they are. Keep your praise — and your criticism — centred on the output, and the situation. Don’t make it personal in either case.
Don’t be a dictator
Developers are opinionated. They like doing things their way, and a lot of the time they don’t like the alternatives (look at vim and emacs). These opinions can become an issue when it comes time for code reviews if there aren’t some established boundaries around how feedback should be applied. I follow the idea that bugs and functionality issues are blocking issues, and I won’t approve a code review while these issues persist. On the other hand, if I raise feedback and it is something opinionated or subjective, It is reasonable for the recipient to reject the feedback in a lot of cases (unless it goes against your code standards or negatively affects code quality).
To effectively apply this point, the language you use in your comments will be pivotal. If someone has done something which you subjectively thing could be better, don’t tell them to change it, instead, prompt them to see if they would consider making a change. They might be open to the idea, and didn’t know the solution you do, so try phrasing your comments like “consider doing it this way instead, because {reason}”, rather than “change this because {reason}”.
If you don’t have code standards around this particular issue, and it isn’t technically wrong, they don’t have to change it, but you can still raise it if you think there is room for improvement.
Everyone has a say
As mentioned at the start, these are my personal preferences, and you might like some of them, you might hate them. This one is pretty important, in any case. Code reviews are a team process. It doesn’t matter how great and well designed your processes are if the people who have to follow them hate doing so. Creating the code review process and setting the parameters of what is acceptable and what isn’t should be a collaborative process. Everyone affected should have a say, and the process should be open to review and revision regularly. Designing the code review process as a team will ensure that you end up with a process which everyone understands, and is happy to follow.
These are just a few principles which I have found to be very useful in code reviews. Generally, if everyone approaches code reviews with mutual respect, and a desire to make the code as good as it can be, the benefits you will get from the whole process will be greater than if everyone tries to push their code-style agendas. Do you have any excellent code review tips I haven’t covered? Please leave them in the comments below!