Opinionated comments, double standards, and more

Photo by Andre Hunter on Unsplash .

Firstly, I want to clarify the title. I don’t hate code reviews when they’re done correctly. In fact, I love reviewing code and being reviewed. With a supportive environment and consistent standards, code review can be a pleasant learning experience for both author and reviewer. Proper code review reduces bugs and maintains code base quality.

But in reality, it is often a stressful exercise. Toxic comments from a reviewer can cause constant arguments. In some cases, it causes a breakdown in team communication. Those are the times when I (and many developers I know) hate code reviews.

Here are a few things that I dislike the most about code review.

1. Opinionated Comment From the “I Am Always Right” Reviewer

In my past working experience, many teams have at least one senior developer with excessive ego and/or arrogance. This person can easily pollute the team environment with toxicity via code review.

An arrogant reviewer will try to prove they are better than others via code review. These reviewers tend to keep adding opinionated comments without justification or explanation. Many times, those issues fall into the category of personal preference.

I’ve seen code reviewers addicted to naming conventions or pedantic with over-engineering. They force the other developers to change their code according to the reviewer’s personal preference, even when the code meets the team’s coding standard.

Due to the perceived authority of the reviewer and time pressure to pass the PR, the other developer often gives in and makes the change. It’s a waste of time and effort and will make the other developer feel demotivated. Often, the unnecessary last-minute change due to the review will cause new bugs.

According to GitLab’s best practices:

“Accept that many programming decisions are opinions. Discuss tradeoffs, which you prefer, and reach a resolution quickly.”

It’s simple and clear. I believe every developer will agree that it’s common sense.

2. Double Standard or a Standard That Keeps Changing

In some projects, there is no formal written code review standard. unfortunately, one or two loud and arrogant senior developers often do all the code reviews. That’s when code review can become a power play instead of necessary quality assurance.

One symptom is that the code reviewer will apply different standards depending on the author of the PR. I’ve seen new or junior developers being put into a complex task alone, followed by higher-than-usual standards in code review. Thus, the PR is blocked and demanded to be rewritten, which causes unnecessary frustration and delays. In the meantime, the PRs from others are given the green light quickly without much proper review.

The only thing worse than double standards are standards that keep changing.

Software design is an art, and there are no fixed rules for art. Instead, we have many principles that are open to interpretation. The changing standard happens when you make code changes according to a reviewer’s feedback, but they keep moving the target. The result is obvious: significant delays and endless discussions with unreasonable people.

From my experience, double standards cause significant anxiety for other developers. They feel insulted and demotivated and eventually start looking for other opportunities.

3. Repeated Comments for the Same Trivial Issue, but They Miss the Core Design/Structure Problem

A common issue is that code review believes “more comments means better review.”

Without spending time understanding the context of the PR, the easiest way of bumping up the number of comments is to pick a minor style issue and add a comment for each occurrence. This isn’t helpful, as a styling issue will normally be fixed by applying an additional/missing setting in the IDE. What helps is to add one informative comment with a link to the style guide or setup guide.

Tunnel visioning on style and trivial issues causes blindness to real problems.

The main structure or design issues often pass the review because the reviewer lacked time and context. The reviewers are normally senior and/or long-tenured staff, so they will likely notice those issues if their focus is in the right place.

4. Out-of-Scope Comments

Some code reviewers get used to making comments like, “I know it’s not your code, but since you’re touching it, please refactor it.”

Every PR has a scope. A good PR has a small one. It either fixes a bug or adds a feature. The code review should be appropriate in scope. It should not increase the scope of a PR by asking the author to do a refactoring just because the PR touches the same file. Doing that will likely cause the other developer to rush and make incorrect changes under time pressure. Mixing two unrelated changes into one PR isn’t good practice.

Unless the refactoring is small and directly related to the PR, it is better to create a separate issue/ticket and do it through another PR.

5. Slow To Review and Respond

Code review should be timely. We all know waiting is painful, and waiting for code review feedback while under delivery pressure is more painful.

In most projects I’ve worked on, there was no SLA for code review time. It depends on how many reviewers are available and how busy they are. It becomes worse when the code review is restricted to one or two senior developers on the team. Because code reviewers are normally senior, they will have multiple responsibilities, making the wait even longer.

It’s not uncommon for code review to become a bottleneck, especially in an Agile environment that requires a deliverable every sprint.

While there is no standard for code review speed, Google’s guideline is probably a good reference:

“If you are not in the middle of a focused task, you should do a code review shortly after it comes in.

One business day is the maximum time it should take to respond to a code review request (i.e. first thing the next morning).”

Final thoughts

The above are the common problems I experienced with code review. It’s not the fault of the code review itself.

It’s simply that the review is not done by the right people or the right way.

You may think, “How should we deal with these problems?” There won’t be a simple answer. I may share my thoughts with you in another article. If you have a similar experience and/or have something to share, please leave a comment. I would love to hear from you.

In the end, I want to share an excerpt from Google’s Code Review Developer Guide :

“In general, reviewers should favor approving a CL once it is in a state where it definitely improves the overall code health of the system being worked on, even if the CL isn’t perfect.”

Happy reviewing!