Make your code reviews a gift

A guide on how to make your feedback a sought-after gift.

Reviewing other people’s code is an integral part of the software engineering process. It helps to catch bugs, spread knowledge, and gives engineers a reason to talk to each other.

These benefits don’t come for free however. None of those good things will be taking place if no one can understand (or care to understand) what you are saying to them. Therefore if you want your reviews to be of any value, you have to keep both of the following priorities in mind while reviewing:

• Ensuring a high level of quality: making sure the code is functional and maintainable.
• Communicating effectively: giving feedback without making the author regret their decision to request a review from you.

It turns out that communicating effectively is quite easy with a bit of effort and empathy. Here are some principles and practices that I’ve found helpful for building that empathy whenever I act on a review request that’s come fresh off the Octocat operated press.

Insufficient context leads to insufficient feedback

Taking the time to ensure that you understand what the code you are reviewing is trying to do helps you give feedback that is direct and actionable. In my experience, I have found this kind of feedback to be highly valued.

Without understanding what problem the author is trying to solve or why they made the decisions that they did, any feedback that you give will be misguided, and potentially conflict with what the author is trying to accomplish. This puts an extra burden on the author to educate you, which leads to additional back-and-forth that slows them down.

You can get this necessary context beforehand by reading any supporting documentation that comes with the review request, such as the pull request description or a Jira ticket. If these things do not exist, you should ask the author to provide the context you need before you start suggesting changes.

I’ve also found pulling down the new code to my local environment and trying to run it in someway to be extremely helpful. Taking it for a spin removes the need to deduce the intended functionality, and can have a nice side-effect of shaking out any lingering bugs that the author may have missed.

If you are using GitHub, you can nudge pull request authors to provide context upfront by making use of a Pull Request Template that can be filled out before requesting a review. A good template consists of at least the following sections:

• A quick summary of the new changes.
• Links to any supporting documentation, like a Jira ticket.
• Instructions for testing the changes locally.
• If necessary, a more detailed description.

Feedback is a gift best served quickly

Most repositories have some sort of blocking mechanism to prevent code from being merged into the main branch without at least one approving review from another engineer. If you happen to be chosen as that other engineer, you will be doing the code author, and your whole team, a real solid by starting to review their code as quickly as reasonably possible. Waiting too long to start delivering feedback can block work on other features that depend on that code, and introduces the possibility of merge conflicts and bugs caused by falling too far behind from other changes that are being reviewed and merged in a timely manner by more dutiful teammates.

It’s best if your team agrees on an “SLA” for leaving a code review to keep things running smoothly and consistently. I’ve found 1 business day to be a reasonable one. If you need help keeping everyone honest to that SLA, GitHub has a nifty Slack integration that you can setup to ping a channel daily with a list of PRs that are stuck waiting for a review.

Get to the point

Once you get around to delivering feedback, it’s important to give it in a manner that gives the code author clear next steps on how they should proceed. When they read your feedback, there should be no ambiguity about what you think is wrong and what you want them to do about it.

Bonus points are given if you proactively offer solutions. Even if they aren’t fully baked, they can help the author further their understanding of your feedback, and kickstart their thinking towards an alternative of their own.

To further illustrate this point of getting to the point, here are two contrasting examples of a piece of feedback about an imaginary and inefficient database query:

• Open to interpretation, and kind of mean: “This database call isn’t necessary.”
• Explicit: “It looks like this same exact database query is already being made in explicitFunctionName(), which is upstream to this function. Maybe the query result can be passed down here as an argument? Like so: [clear and concise code snippet demonstrating how this potential change would work, so clear in fact that it might even been able to be copied and pasted directly with no modifications]”

As the code author, receiving the explicit feedback upfront means I can avoid needing to follow up with the reviewer to understand why the database call isn’t “necessary”, and can instead start implementing the necessary changes immediately. And as the reviewer, I can avoid needing to spend additional time further explaining my feedback, and can instead move on to other aspects of my life. It’s a win-win!

Show your work

When suggesting changes, you can further get to the point by explaining your rationale behind them, and providing code examples proving that they would work. Doing this shows the author that the feedback isn’t just talk, but actually viable to implement. And perhaps most importantly, people will take you more seriously if you showed them that you used your brain.

Showing your work also applies if you end up not needing to leave any feedback at all. As an author, knowing that your code has been given a good and thorough rundown helps to instill confidence that is is in fact, good. While receiving a review with nary a peep does feel nice, it’s not immediately clear if that good and thorough rundown ever took place by the reviewer. I like to remove this ambiguity by leaving a comment along the lines of the following:

“Nice work! I tested it out on my machine and it worked like a dream! 💯💯💯”

Think before you speak

Tried and true advice, applicable even in the realm of asynchronous code review, where words are rarely, if ever, physically uttered. Every piece of feedback you give is extra time the author needs to spend digesting and taking action on. So, it is worth taking a second before leaving feedback to consider if it would be materially helpful towards getting the code merged, or is just extra noise.

This mostly means omitting feedback on the subjective aspects of code. Such as:

• Formatting. This should be handled automatically by developer tooling and checked in CI, using tools such as Prettier and/or ESLint.
• Alternative implementations that are equivalent in functionality and complexity. There are often multiple valid solutions when programming, and the author has already invested the time to implement the one you are looking at.

It also helps to re-read your feedback before delivering it to make sure it is coherent. In GitHub, you can batch multiple comments in a single review, which gives you the opportunity to review your review (heady!) before it is seen by the author, and ensure that all of your comments make sense and don’t conflict with one another. I often find myself re-working or deleting comments that sounded great when first writing them, but upon reading them back are either nonsensical or simply aren’t helpful.

Ask before you command

People are way more receptive to requests when phrased as a question rather than a command. It’s true, look it up.

Framing a suggested change as a question prompts the user for a response, which engages them to participate and think about how to best implement it. Commands, on the other hand, shut down productive conversation and close off avenues for thinking critically about alternative solutions. Consider the following example:

• Command: “This should be its own function.”
• Question: “How would you feel about splitting this block off into a new function? I could see us needing this functionality somewhere else in the future.”

Asking the author what they think about the value of extracting a function, along with explaining why they’d want to do that, encourages them to think about how they can best design a function that can be easily reused. Issuing this feedback as a command might result in either a function that isn’t as well designed, or the author assuming a defensive posture to avoid changing their code. At the very least, you definitely won’t be making any friends with that approach.

And who knows, maybe by asking questions you may even end up learn something new for a change!

When in doubt, talk it out

It’s easy for misunderstandings and stalemates to occur over the course of long, drawn out conversation threads. When this happens, it’s best to hop on a call with the other engineer to get them sorted out. Sometimes it’s just easier to resolve conflicts by talking them out, and hearing a real human voice on the other end helps to build empathy for the other person and better frame what they are trying to accomplish.

Code review is as much of an empathy building exercise as it is a code building one. Like how that one song goes, we only get what we give. Approach code reviews with a mindset of wanting to be helpful, and you may very well be so.