Top 5 Blunders Made by Code Reviewers

moozzyk

Pawel Kadluczka

Posted on May 23, 2024

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

Image description
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. Check out my post: 7 Tips To Accelerate Your Code Reviews


💙 If you liked this article...

I publish a weekly newsletter for software engineers who want to grow their careers. I share mistakes I’ve made and lessons I’ve learned over the past 20 years as a software engineer.

Sign up here to get articles like this delivered to your inbox.
https://www.growingdev.net/

💖 💪 🙅 🚩
moozzyk
Pawel Kadluczka

Posted on May 23, 2024

Join Our Newsletter. No Spam, Only the good stuff.

Sign up to receive the latest update from our blog.

Related

How to effectively work in big codebases
softwareengineering How to effectively work in big codebases

August 15, 2024

Top 5 Blunders Made by Code Reviewers
softwareengineering Top 5 Blunders Made by Code Reviewers

May 23, 2024

Future proof code
softwareengineering Future proof code

January 12, 2024