The Responsibility of Code Reviews

As a code reviewer, you are are the vanguard to the longevity of the codebase. You are responsible for making sure a merged pull request does not introduce code smells, unreadable code blocks, and untested features. If not for you, the codebase could quickly devolve into an unmaintainable mess. A nightmare not only for you, but costly and counterproductive to the company for which you're paid to provide value.

Hopefully you're not alone.

Obviously, mistakes will be made. It's why code reviews are important in the first place. It's also why even after pull requests have gone through code reviews blemishes find their way into the codebase. Code reviews aren't just about blemish-prevention, they're about encouraging a shared understanding of the codebase.

Consider a codebase whose updates do not go through reviews. Let's also suppose there are only two engineers making such updates, and they alternate in which features they develope and patch. Each time they start a new feature or patch, they will need to read through and understand the other developer's code. Perhaps they will need a meeting to understand an aspect of it. Perhaps they will notice something isn't working as intended or an important edge-case was not considered. These conversations would have happened if they had done a code review, but now it is happening in a much more sensative context. The engineer who found the problems may be frustrated because they haven't been able to start their work yet, and the other engineer may feel defensive for having their previous work so judged.

In the supposed situation above, had they reviewed each other's work in a context where mistakes are expected to be found and where constructive criticism is welcomed, not only would the engineers be engaging in a team-building exercise, but they would inadvertently be sharing with each other how the codebase is growing. There knowledge of the codebase would have much more parity; they would have knowledge of what they worked on, and knowledge of what they reviewed. Ultimately this lets the codebase be more welcoming to new engineers and ease the pain when an engineer ultimately leaves the project.

Okay, so code reviews are important. Probably you already knew this, if not in detail, then you knew it conceptually. But how are they done? How can a code review go wrong?

First its important to establish with your team what you are all looking out for in the reviews. Generally you would want to use some kind of linter (like prettier and eslint) to normalize the code before it gets commited in the first place. That way you and your team can focus on things like catching bugs, code smells, or enforcing architecture or naming patterns.

Once the above is put in place, you're already 90% there. But that is all the easy part. This last 10 percent is where things can hurt team dynamics. The last, and biggest, thing to keep in mind is how to keep the criticism in the review professional, productive, and constructive.

Receiving cricism is a practiced skill as much as giving it, so when you're writing the review you should make sure you're professional in the feedback. Remember you're there to make sure the code is up to the team's standards, not to take down your colleagues if they make a mistake.

Your feedback should only address what you think could be done better. If it's an obvious typo or some other kind of small error, just make a small note. To use a metaphor, a polite beep in traffic to make sure the other drivers see you. If the error is with respect to a large chunk or logic or how the code was organized, phrase your feedback so that it facilitates a conversation: "I had trouble following this, could you explain what these modules are doing?"

In the rare case that the pull request is beyond a simple review and it would be much too time consuming to review, then I suggest reaching out to your teammate directly. It would be better to give the developer the benefit of a doubt then to leave a publicly scathing note. If their pull request is missing a chunk of local commits they forgot to push, then your commit would appear in very poor taste and leave your colleague feeling less eager to work with you in the future. By reaching out directly you're starting a conversation about the code. Indeed, if the code to be reviewed is in such a poor state, then clearly something has been misunderstood. Even a simple call or chat are better mediums then a pull request for that conversation.

Keeping the review productive should not go unconsidered, either. Your team has other priorities, other code reviews to conduct, and more features to build and bugs to squash. When you're thinking about feedback to give to your teammate, consider how important it is that you both have a conversation about that module or line of code. Remember that they don't need to code exactly like you. Is your concern personal? Perhaps the code fine as-is, just not how you would go about writing it. If it's a borderline case, I will usually say something like, "This is just a note. You don't have to change this, but this is how I would go about it: ..."

In short, code reviews are an invaluable resource. They are a process by which an engineering team can facilitate long-term health of the codebase both by protecting it from cumulative blemishes and by enabling the team to collectively understand its disparate pieces. Ensure the reviews are professiona, productive, and constructive, and your team will be happier and your software will prosper.