Top 5 Blunders Made by Code Reviewers
Code reviews are a standard practice in software development. Their purpose is to have another pair of eyes examine the code to catch issues before they affect users and to provide feedback that helps make the code cleaner and easier to understand.
While the goals of code reviews are noble, the experience for many developers is often less than stellar. There are many contributing factors, but one that stands out is how code reviewers approach and conduct code reviews.
Here are the top 5 mistakes I've seen code reviewers make (and I had made myself) that left fellow, often junior, developers discouraged and frustrated with the code review process.
Approving a PR without understanding the change
Many developers approve PRs very quickly, looking at the code only barely or not at all. While the speed matters, this attitude leads to problems:
bugs that could be identified during code reviews are missed, reach production, and impact users
the quality of the code deteriorates over time, making implementing new features more challenging
both the code reviewer and the author miss the opportunity to learn something new from the PR
Proper code review requires time, effort, and, often, additional context. I sometimes realize that a change I started reviewing requires more time than I can afford or that I don't have enough context to understand it fully. If this happens, I will still review the change as best as I can, but I will let the author know that I can't sign off on it.
"If you approve it, you're responsible for it" was one piece of advice I received that completely changed my perspective on carelessly approving PRs.
Unprofessional feedback
Code reviews should be all about code. Sadly, they sometimes become personal attacks with harsh or condescending comments. This kind of "feedback" usually extends beyond code reviews and leads to a toxic team culture.
Even if the code sent for review has multiple issues, comments like: "this code is s**t!" are not helpful. Explaining the problems and suggesting solutions is a much more effective approach. Talking to the author is even more effective.
Too much focus on less important details
Flooding a PR with nitpicky comments is not good feedback. Not only is it borderline passive-aggressive behavior, but these comments can also drown out ones that raise important issues.
One great example is comments about code formatting. Asking the author to adhere to the Coding Style Guidelines adopted by the team is one thing, but commenting on each single incorrect indentation or misplaced parenthesis is not OK. Fortunately, this entire class of arguments can be easily avoided by integrating a code formatting tool. Not only will the tool end the petty arguments about code formatting, but it will also allow developers to focus on what's important. (I wrote about this in more detail in this post: The downsides of an inconsistent codebase and what you can do about it.)
Unclear or unactionable feedback
Comments like: "I am sure it can be done better" are not useful. They leave the author clueless about the reviewer's expectations and the improvements they expect. In the best case, the author will ignore the feedback. In the worst case, they will try guessing what the reviewer meant and iterate on the code, often unnecessarily.
From my experience, illustrating comments with code suggestions is one of the clearest and most effective code review feedback.
Delaying code reviews
One of the most common complaints about code reviews is that they significantly slow software development. The most common reasons are:
reviewers are not picking up PRs for review
reviewers are not responding after the author addressed the feedback
changes are flooded with comments on minor issues, and resolving them requires many iterations
Assuming a PR is not intentionally blocked due to a serious concern, delaying reviewing it can be frustrating for the author. Often, reviewers are simply busy with their work and don't have time to review someone else's changes. But this is a double-edged sword—eventually, they will want someone to review their changes, and they shouldn't expect quick reviews if they don't review PRs promptly.
Sometimes, it is a matter of being better organized. Blocking half an hour daily on the calendar for code reviews should help team members move faster.
That being said, if your PRs are not getting reviewed, there may be something you can do about this. Checkout my post here: 7 Tips To Accelerate Your Code Reviews.
If you found this useful, please share it with a friend and consider subscribing if you haven’t already.
Thanks for reading!
-Pawel