Avoiding 3 Common Code Review Mistakes: Lessons Learned
Learn from my experience to avoid three common mistakes in code reviews, enhancing your team's efficiency and code quality.
It was a normal day at work until I found out I’d be in a room with my manager and my not-pleased teammate about how I was reviewing code.
It was not a fun situation to be in.
I was reviewing code entirely wrong.
When I joined Gusto, I had come from an incredibly fast-paced startup. In that startup, we were brutal with code reviews.
Feedback was harsh; like “What the heck are you doing” and “We must do it this way.” We were all close friends that knew each other well and knew each other’s intentions. But that’s not most working environments.
I knew when I joined Gusto I had to tone down the harshness and directness, but I still made a few key mistakes that led to the uncomfortable manager meeting.
This was effectively what I was doing before 🤦♂️ 😬
Ok, maybe not this extreme, but you’ll see what I mean.
Let’s jump into it.
🤦♂️ Mistake #1: Code must be written “my” way
When reviewing code, I would write comments that enforced my particular style.
In reality, everyone writes code differently.
Stylistically, the comments should be more about matching the existing code, rather than how you write code. Now, if you ended up writing all the existing code, that’s a different story 😅.
Here’s an example:
Let’s say someone is declaring a constant at the top of the file. The rest of the codebase uses SCREAMING_SNAKE_CASE but your teammate wrote in PascalCase.
This is a great opportunity to let them know the rest of the code uses SCREAMING_SNAKE_CASE and they should match it.
The problem arises when there’s no existing convention and I would start enforcing my style. It might be saying that we should declare constants in a certain place, putting helper functions at the bottom of a file rather than at the top, etc. In general, I’d be overly nitpicky about things that were not worth either of our time.
🤦♂️ Mistake #2: Instructions vs. Suggestions
The way I was viewing code review was damaging my relationships.
The way I do it now helps build stronger relationships with my teammates.
At the time, I thought of myself as a goalie 🥅 , preventing any code I thought wasn’t 1000% perfect from going into the codebase. This mindset led me to “instruct” my teammate to make any changes I suggested, leaving very little room for discussion.
Now, I see myself as a blocker whose goal is to unblock as early as possible. I trust my teammate to make the right decision given the information, and if they would like my opinion, I’m available to provide it.
This mindset shift led to using suggestions instead of instructions.
Here’s how the difference looks.
Before (instruction):
The response I’d get was usually defensive—“I’d rather keep it here.”
After (suggestion):
This creates a collaborative conversation. When I ask like this, the response is usually, “Great idea! Let me do that.” Or, “I’d love to, but when doing that I ran into this issue. Do you know if there’s a good workaround for that?”
You may be wondering: “But what if the comments don’t actually get done if I’m less forceful?”
For that, I’ll say 3 things:
I’ve found that comments get done more often when you don’t put your teammate on the defensive.
When you really need something to be done a certain way, you can put a bit more emphasis on it, separating it from your “collaborative” comments.
It’s ok if it doesn’t get done! Your goal is to unblock and collaborate. Focus on the benefit of creating a stronger relationship and capital for future code reviews, both ones you receive and give.
Here’s a good rule of thumb:
70% of the time: I accept changes with or without comments.
25% of the time: I submit a review with just comments and questions
5% of the time: I request changes. Reserved only for bugs or dangerous changes
🤦♂️ Mistake #3: All comments were blocking
Finally, I was mistakenly viewing every comment as required.
In reality, each comment has a different level of severity.
Now, for any stylistic or optional suggestions, I’ll add “nit:” to the very beginning.
This makes it clear to the reviewer I know I’m being nitpicky, and they can choose to accept the suggestion or not do it. Most of the time, my code reviews are approvals with a few “nit” comments like this.
Here’s how it looked before:
And here’s how it looks now:
It’s a small difference. But over many comments which can become nits, it helps your teammate in the following ways:
Even though there may be 10 comments, it avoids affecting their pride since they see you just view them as nitpicks rather than, “10 blocking comments.”
It builds a connection that says, “Just here to work together on making this the best we can,” instead of, “All of these must be fixed to merge your code.”
Disclaimer: With all of this advice said, each company’s environment and culture will be different. In the first startup I was in, everything I was doing was perfectly normal and expected. Take from my experience what resonates with you most.
Lastly, I’m also not saying that the “bad examples” presented are always bad. Sometimes they make sense. It depends on the context, your relationship with the other person, and the culture.
📖 TL;DR
Code review should be viewed as…
Matching existing code or newer patterns
Suggestions, not instructions
Raising potential edge cases or bugs
Code review is not a means to push your own stylistic patterns on everyone
I’m considering doing a post of everything you should do during code review, including simple swaps in wording to make your comments land better. If you’re interested, comment below to let me know!
As always, thank you for reading.
- Jordan
P.S. You can reply to this email; it will get to me, and I will read it even if I can’t always reply in a timely manner.
Did you find this issue valuable? If so, there are two ways you can help:
You can also hit the like ❤️ button at the bottom of this email to help support me. It really helps!
Great post, reminds me how important soft skills are in software engineering
What not to say in reviews. Kiwi edition.
https://twitter.com/rickysullivan/status/1626017850296250372?s=20