Skip to main content

Command Palette

Search for a command to run...

Most teams are doing code reviews wrong. Here are 3 warning signs, and 3 fixes.

Published
4 min read
Most teams are doing code reviews wrong. Here are 3 warning signs, and 3 fixes.
C

Software engineer; solopreneur. Writing to help developers level up. Follow me on Twitter for more.

Software engineers engage in code reviews daily.

Some teams have a great code review process. Others could use improvement.

Here are 3 signs of a bad code review process, and how your team can ship faster.

3 signs of a bad code review process

🚩 If code reviews make you, or other developers, anxious.

Likely, reviewers aren't prioritizing kindness. Or they're on a power trip, gatekeeping over minutiae.

The result? Deteriorating relationships.

🚩 If it's common for one pull request to go through many review cycles — maybe even 7 or more.

Likely, reviewers are giving ambiguous feedback. Authors don't have a clear path forward, and go down rabbit holes between reviews.

The result? Discouragement, and wasted time.

🚩 If there's a lot of back and forth — if lengthy discussion threads are common.

Likely, the team isn't equipped to resolve disagreements. Or the team isn't aligned on nuanced readability principles.

The result? Delayed shipping.

wrong-way.jpg Photo by NeONBRAND on Unsplash

I've been on a variety of software engineering teams, in and outside of FAANG. I've seen the above come into play over and over again. It's a problem industry-wide.

One-off occurrences of the above are nothing to worry about. But if they're common, your code review process needs improvement.

It's easy to blame individual code reviewers. But to implement a solution that lasts, these problems should be solved at the level of the team or organization.

But where do we start?

First, understand that some aspects of code reviews are objective — finding flaws and defects.

But most aspects are subjective — opinionated discussions around software quality and readability.

Subjectivity drives inevitable disagreements. So we need to establish an environment that effectively handles these disagreements.

3 keys to a good code review process

Here are our goals:

  • 1️⃣ Encourage healthy disagreements
  • 2️⃣ Proactively prevent disagreements
  • 3️⃣ Methodically resolve disagreements

But how?

It starts with documentation: team-level code review guidelines.

1️⃣ Encourage healthy disagreements

Document code reviewer expectations on how to give feedback.

When I was a software engineer at Amazon Web Services, guidelines for my teams included:

  • Be kind
  • Comment on the code, not the person
  • Prefix nitpicks with "nit"
  • Give a reason why, and a clear path forward

2️⃣ Proactively prevent disagreements

There are all kinds of readability principles out there. "Functions should be small" and "class names should be nouns."

Even the best programmers disagree on those, and other principles of readability. Chances are, your team will too.

So, document your team's stance on the nuanced principles.

For example, here's the Gumroad team's stance on the popular principle, "Don't Repeat Yourself." Credit goes to the Gumroad engineers for this one. 👇

"Repetition is better than the wrong abstraction.

There is nothing wrong with repetition. Approaching DRY (Don't Repeat Yourself) as a maxim will lead to code that is difficult to adapt to changes, and it will increase cognitive burden. Repetition can be consolidated more easily than the wrong abstraction can be untangled. Do repeat yourself until a clear pattern starts to emerge in a handful of different places. Think about whether their similarity is coincidental."

You may agree or disagree with that statement — that doesn't matter. The team came to a decision as a unit. This influences future contributors of the codebase, and prevents many subjective discussions on the topic.

3️⃣ Methodically resolve disagreements

Document how the team should drive disagreements to a resolution.

Verbal conversations work great. Establish a final decision maker to handle an impasse between two developers.

  • Engineering discussion ➡️ Tech Lead
  • Product discussion ➡️ Product Manager
  • Design discussion ➡️ UX Designer

So, let's recap!

3 signs of a bad code review process:

  • 🚩 Feeling anxious
  • 🚩 Many review cycles
  • 🚩 Back-and-forth

3 keys to a great code review process:

  • 1️⃣ Encourage healthy disagreements
  • 2️⃣ Proactively prevent disagreements
  • 3️⃣ Methodically resolve disagreements

Empower your team to ship better software, faster. 💡

Like this article? It's an excerpt from my upcoming video course!

Master the Code Review

  • 1️⃣ Write better code
  • 2️⃣ Give better reviews
  • 3️⃣ Forge a better process

Code reviews are critically important. But there's limited content on how developers can succeed in a code review environment. I'm looking to change that.

📬 Subscribe for a free lesson, and discounted pre-order announcements!

👉 https://curtiseinsmann.com/

paypal-diff-view.png

U
uzlopak4y ago

Actually no real advice was given by this article. Also Lacks of hacker mentality. If you have a better idea on solving an issue in a PR then provide a poc or branch off and make your fixes there. The original coder can then adapt it by merging and then there is less back and forth. Or atleast provide pseudocode and/or offer pair programming.

I already had cases, where the team thought a solution was good enough, despite my "nitpicking". I finally branched off, added a poc exploit and branched again off and fixing it properly...

Last advice has to be, like Aristotle already wrote, if you have somebody, who is not able to discuss, than don't discuss with him. It means in our world, to leave the team or maybe the company.

C

The article specifically calls out 3 things that should be documented in the code review guidelines. If they were unclear, I’m open to suggestions on how they can be clarified.

A code reviewer isn’t expected to fix the author’s code. They’re reviewing, not doing the work for them.

1
C

Great article. Shameless plug but I actually wrote an article outlining some tips from a code review author's point of view, which touches on some of the same points.

C

Thanks for sharing! Respectfully though, I don’t see where they touch in the same points, other than the fact they’re both about PRs.

V

Thanks for sharing this informative article :) Communication is definitely key to ensuring an efficient and healthy code review environment.

1
C

Happy to share! Thanks for reading. 😄

P

Great article! I would share this with my reviewer LOL

1
C

Please do!

T

Nice one Curtis Einsmann! Great advice - I definitely agree with you on these points. 👏

I always new that code review is sensitive matter, but only today it occurred to me why that is - it all revolves around creating and resolving disagreements.

1
C

Yes it’s a daily process where there’s a lot of opportunity for improvement in many teams. Thanks for reading!

1
B

Great content, Curtis! It’s not uncommon to see these unhealthy code reviews on the job. Thankfully, we can all do our part to try and change the feeling and effectiveness, and promote a healthier culture.

2
C

Well said. Thanks Braydon! 💯

More from this blog

Curtis Einsmann's Hashnode Blog

9 posts