How These Code Review Etiquettes Can Shape Your Team Culture

Have you ever felt daunted by your code being reviewed? Have you ever felt a lack of confidence in reviewing other people’s code? If you have specific negative emotions facing the code review process, maybe it is time to look at how people on your team have been doing code reviews.

Especially in a company with rigid leveling in engineering, which creates a power hierarchy, code reviews can be a passive-aggressive tool to undermine people and ultimately hurt team morale. On the other hand, if everyone on the team has a positive attitude and upholds etiquettes, code reviews can be a powerful tool to bring up the overall technical skills and strengthen the team.

Goals of Code Reviews

Why do software engineers do code reviews before the code is shipped? The obvious reasons are the following -

1. Catching obvious logic errors
2. Making sure testing is sufficient, which should unit tests, integration tests, manual tests, etc.
3. Pointing out inconsistent code styles

Code reviews have not so obvious reasons, too -

1. Providing documentation for the past decisions on specific code changes, especially for people new to the codebase.
2. A communication tool for engineers to make coding decisions. (However, not design decisions)
3. A forum to encourage people to learn from each other

Many online resources and resources at your work can help us achieve the obvious goals. In this article, I will talk about how engineers do to achieve those not-so-obvious goals of the code reviews.

Good Intentions

Good attitudes direct how we behave. It is a good start.

Be critical about code, but patient with people

Many years ago, I had the pleasure of working with a Senior Principal Engineer at Amazon. When we were discussing design questions, he never appeared to me as being arrogant, showing off his level, or belittling my idea. He was the most patient engineer I had ever met. When I talked through my thought, he was always listening and considering and discussing them. He never hesitated to compliment me when my idea was good. Over the year I realized that the maturity of an engineer is not only measured by the technical talent and experience but also how the engineer works with others.

However, entry-level engineers and interns are often the targets for ruthless code review comments. These engineers have just left college, with no or limited industry experience. Their code may not be easy to meet the production standard immediately. Their first code reviews are often the ones that are potentially bombarded with criticisms. More experienced engineers are tempted to leave over 50 comments on the first code review. I have seen or heard this happened many times in various workplaces. Do they know the coding best practices now after the code reviews? Yes. Do they feel good at accomplishing something? Probably not. It is embarrassing when people are pointing bad practices and mistakes all over the place, in front of the entire team.

For new engineers, they don’t know what they don’t know. I have learned to sit down with them and do an in-person review for the first one or two code reviews.

Be fair and inclusive to all different groups of people

Wayne State University researchers have discovered in a same-gender and cross-gender interaction study that, female engineers less frequently wrote negative comments to their male counterparts, but male engineers from 3 out of the 6 projects were not only critical of their female counterparts but also withheld positive encouragements. They also found males more frequently using expletives or words that are demeaning to females.

In another study, after analyzing nearly 1.4 million users of Github, the US researchers found that code changes suggested by females had higher approval ratings than those from males — but only if their gender was not identifiable.

A similar bias happens to engineers with different levels, too. Anecdotally, I have seen engineers took suggestions or responded to comments if the reviewers are with higher levels. If this reviewer is higher than the author, her comments are assumed to be good or better. On the other hand, if the reviewer is lower than the author, her comment is assumed to bear less weight.

To junior engineers, I encourage them to review senior engineers’ code, because humans make mistakes, as simple as that. Attention to detail does not require 10+ years of experience, but a pair of attentive eyes and a curious mind.

No matter whether the reviewers are higher or lower than my level, I always take them seriously. I read every word, and reply to them. My reviewers spend the time reviewing my code, and my replies are at least to respect their time.

Be humble and appreciative

Heated arguments in code reviews often come from the ego. When an ego clashes with another ego, defensiveness comes.

For example, a junior engineer could point out that a senior engineer missing a unit test case. This senior engineer thinks, “um, how dare you point this to me? how much do you know?” The senior engineer starts to act it out, or ignore the suggestion. When emotion is involved, it prevents engineers to think objectively and critically about whether the comment is valid or not.

Before your comments are submitted, review the comments, and see whether it shows the ego. Is there any of the language that can be interpreted as “you have to listen to me”, “I’m the best”, “I have the authority”, or “what you do is bad”.

Good intentions are not enough. They’ve never put an onion in the soup yet. — Sonya Levien.

Etiquettes in Action

Words are easily misinterpreted. Good intentions can end up with bad outcomes. If you don’t know how to start, here are some tactics. Practice and slowly observe the change in code review dynamics.

As a reviewer

Start with “We”, and be cautious with “You”. Unless you work alone, the codebase is always co-owned by the team. Any code change is not my own decision but a reflection of a team’s joint decision. This is why we have code reviews. Use “We” to show we are a team.

Okay -

“You should add more tests cases here.”

Better -

“We could add more tests for this method.”

“It would be helpful to add more test cases for this method.”

Present the comment as a question or a suggestion, instead of a statement. A question or a suggestion welcomes a conversation, rather than sounding like a decision that has been made, by you. If you raise a question, the ball in the author’s court to consider whether this is a good question. The author would need to think about how to answer this question through a critical thought process. If this question is Yes, both of you are on the same page. If No, the author would need to come up with good reasons to back it up.

Okay -

“Add more test cases here.”

Better -

“I would suggest adding more test cases for this method”.

“Should we add more tests for this method?”

Voice support for ignored reviewers. Junior engineers or new hires often face this challenge when reviewing senior engineers’ code. It takes courage for them to review, but sometimes they are worried whether their comments are valid. Sometimes when a code review has concluded without their comments addressed, they would feel discouraged for future code reviews.

If you notice another reviewer’s comment is good but is left out by the author, graciously call it out. It shows support to other engineers and encourages them to speak up more, and also reminds the author of the importance of the comments.

Example -

Sarah raised a good point about using val here. Can we please update this as well?

A fond memory of mine — I joined a new company and started to leave comments on other engineers’ code reviews. My comments were left unaddressed, but one time an engineer pointed it out in the end—

“Grace raised a great point. Please address Grace’s comments”.

I felt my voice being heard and amplified. Since then, I was inspired to pay attention to the marginalized reviewers and vouch for them.

To make sure everyone is aware of this, one team I have been with took this to the extreme — “no comment is left behind”, meaning the author has the responsibility to answer any comment before the code is submitted, even after this code change list has been granted with a critical ship-it sign-off.

As an author

Confirm the good comments. Be generous of compliment. It encourages the reviewers to continue to provide good comments and also shows your understanding is on the same level, which is a confidence boost.

Even for reviewers who may have exhibited passive-aggressiveness in code reviews, your appreciation shows maturity and helps them feel a sense of partnership.

Example -

Good catch! I appreciate this level of attention to detail. Thanks!

Good callout! I will update it here.

I like this angle you pointed out. However, this would not be applicable to this use case because …

Clarify comments offline as much as possible and summarize the decision afterward. Sometimes, an extremely long clarifying chain on the code reviews looks like arguing, even though, in fact, it is nothing but to clarify some details. What makes it look like contention is that the reviewer may think the author does not agree and try to challenge and embarrass the reviewer by asking a lot of questions in front of the whole team. The back and forth question answering chain is also not efficient. A phone call or a Slack chat in a private channel can quickly resolve the questions. Later on, after both of you reach the agreement, update with a comment for visibility to the team.

Ok -

What do you mean? Can you clarify?

Better -

Sarah and I discussed offline. Here is what we decided to do …

Disagree with open doors. It is often you may disagree with the changes that reviewers have proposed. Express the reason why you did it that way, and also welcome the reviewer to chat if she still disagrees. The provided reason shows that you considered the comment and think critically about it. Open to chat opens the opportunity for more discussion.

Okay -

“I don’t think it necessary.”

Better -

“In this case, I don’t think an extra check is necessary because … I’m happy to chat if I missed anything.”

One may argue, “these etiquettes are out of overthinking”. I believe people in the same team have different sensitivity to our actions. These etiquettes encourage us to be considerate and mindful of others.

One may also argue, “if I overthink my comments, it would slow me down”. Yes, it will at the beginning. Once it becomes a habit, it will not slow you down, but make the code review process efficient, because people align and make joint decisions faster.

Toxicity is contagious. Kindness is also contagious. Be the one who is the first to be kind and mindful, and let it spread to the people around you. Have faith in people and people will change.