Avoid these mistakes reviewing code
5 common mistakes when reviewing code and tips to solve.
Get the free AI Agent Building Blocks ebook when you subscribe.
There are two groups of people. Those who enjoy learning in Code Reviews (CRs) and those who perceive them as a painful, hostile environment. I think the latest is a bigger group.
“It’s a feature, not a bug”
We tie our perceived self-worth to our work. We don’t want to know the mistakes we have made. We even rationalize bugs as features.
Let’s explore common mistakes I have seen and better ways of handling those situations.
(This post considers CRs as an asynchronous process. The author sends a CR and receives comments from reviewers. This is iterated until reviewers approve and the change is merged)
Mistake 1: Writing ambiguous comments
Reviewer: “Can we use this library? (link)”
Author: Yes.
Reviewer: Why didn’t we use it?
Author: We only use a small part of the library but it increases our js bundle size.
Reviewer: Isn’t it better than handling the edge cases?
Author: We have testing for that.
Reviewer: But you have a bug in line 43, I saw it yesterday when I posted the first comment
Author: You could have started with that…
The error
The author overthinks what this question means. "Is it a genuine question or a rhetorical question to make me think?". "Is it a blocker or not for the approval?"
A rule of thumb is to avoid ending the comment on a question mark. It’s good to frame as questions and not commands, but don’t end there. Narrow down to where you see the problem and provide the necessary details without overwhelming.
Most times we don’t know what we want and just end the comment with "Thoughts?" to push responsibility to the author. It requires effort upfront to structure our thinking but it saves time in the the long run. The author is responsible for the code under review, but not for clearing your mind
Prevent the author from being blocked and prevent a ping-pong thread of comments, none of them adding any value.
As engineers, we are obsessed with minimizing system network roundtrips that take milliseconds but ignore human communication roundtrips that take hours.
The solution
Use semantic comments. Put a label in each comment indicating what you mean. It can be as simple as "Nit", "FYI", or "Blocker".
Even better, use a convention others can reference, like conventional comments.
Don’t try to get the entire team to follow your new rules. Start doing it yourself and you’ll notice people asking.
A coworker of mine started using them just because he saw the tags in bold at the beginning of all my comments (e.g: [Suggestion - nonblocking])
Mistake 2: Writing extremely concise comments
Reviewer: Refactor this
The error
The author is overthinking: “How do I know if the next changes will solve the comment or not?”
We think others can read our minds. They can’t. Document in writing your reasons, arguments, and what you expect.
The solution
Write generous comments. Include code snippets and references.
From: "Can we simplify this?"
The author has to investigate some way to do it, without ever knowing if it will be enough for the reviewer. There isn’t a success criteria.
To: Can we simplify this using the spread operator of JS? <Code Snippet> <Link>
The author can see a picture of what success looks like and can go to the right source to learn about it.
Don’t put lengthy examples to demonstrate your opinionated approach is better. That’s just being pushy and can be perceived badly.
Mistake 3: Making comments about everything
A new email arrived: “John Foo wrote 37 comments in your Code Review”