How to overcome the common etiquette breaches in code reviews
Code reviews are a big part of writing software, especially when working within a team. It seems that the agreed-upon etiquette need to exist for reviewing code within a team.
A code review is a critique and a critique can often feel more personal than the code writing itself. A sloppy, under-researched, or insensitive code critique can cause difficulties between team members, reduce overall team productivity, and diminish code quality over time. Harsh code reviews hurt feelings.
Tim Wood, the creator of Momentjs said:
"Seeing bugs and issues continue to roll in and being mentally unable to address them has led to feelings of failure and depression. When looking at the moment project, I could only see the negatives. The bugs and misnomers and mistakes I had made. It led to a cycle of being too depressed to contribute, which led to being depressed because I wasn't contributing."
There are many online comments, posts, and tweets by prolific engineers expressing that their feelings have been hurt by code reviews. This doesn't directly mean that reviewers are trying to be mean. Feeling defensive is a normal, quite human reaction to a critique or feedback. A reviewer should be aware of how the pitch, tone, or sentiment of their comments could be interpreted but the reviewer. — see Occam's Razor.
Although reviewing code is very beneficial, a poor or sloppy review can have the opposite outcome. Avoid criticism without providing context. In other words, take the time to explain why something is wrong, where it went wrong, and how to avoid the mistake moving forward. Showing this level of respect for the reviewee strengthens the team, improves engineering awareness, and helps to provide agreed-upon technical definitions.
So I'd like to briefly define some common mistakes, with the tips for improving a code review process.
- Usage of the person
Without realizing it, engineers can neglect the difference between insightful critique and criticism because of personal relationships in communication.
And here is a tip:
Don't use «You» or «We»:
Using you or me is probably not offensive intentionally, so don't worry. However, over time, involving the person can start to feel less positive—especially if vocal tones are ever added.
"You should return out of this function early"
We: Using we is inclusive and a safe way to say something more directly without making someone feel defensive. However, if the person speaking says we, and has not worked on the code at all, it may seem falsely inclusive and insensitive.
"We should return out of this function early"
Use no personal reference: Without the personal reference, conversation or review will closely communicate the problem, idea, or issue.
"Return out of this function early"
Notice how the amount of text needed to communicate the same thing without using personal references takes fewer words and has greater clarity. This helps with human interaction, separates code discussion from the personal discussion, and fewer words are needed to communicate the same idea.
- Usage of the passionate conversations
Passion is an important motivator for improving. The passion that is critical in nature can be very considerate and motivating. Feedback that is critical in nature is most useful if the person receiving the critique is engaged. This sort of communication comes up a lot during architectural conversations or when discussing new products.
Feedback that is critical in nature is most useful if the person receiving the critique is engaged. Note: the person receiving the information must be engaged with the critique.
"There are 8 web fonts used in this mock which may affect page load speed or even certain tracking metrics that could be caused by new race conditions!"
The similar comment, even terser but stated with a calm demeanor, slower delivery, and a normal vocal volume — followed by a question.
"There are 8 web fonts used in this mock. This will affect page load speed and possible tracking metrics because of potential race conditions. How can this be improved?"
Notice how the comments above are almost the same. The second comment is even more direct. It states a problem as a fact and then requests feedback.
An important thing to remember when being passionate is taking on a quieter tone. This is a physical decision — not a social one. Passionate language can be the same, and perceived very differently, based on the orientation of the communicator's tone. If physical tone (body language), vocal tone, vocal pitch, and vocal volume remain gentle, it is observed that it is much more likely for an audience to remain engaged — even if the critique is critical in nature.
- Reviewing the author and not directly code itself
The act of pointing, within written conversation or actual body language, in almost any situation is not optimal for good communication. It changes the focal point of the conversation from the context of the conversation to a person or a thing.
The response below provides a comment and then a link. In the context of the code review, the second part of the comment and link takes the reader out of the context of the code review, which is confusing.
The comment below provides a comment, and then a pseudo-code suggestion.
In the two examples above, the first example causes the reader to go far beyond the issue. The conversation is more abstract—even existential. The second example refers directly to the issue and then provides a pseudo code snippet that relates directly to the comment.
It is best to only comment on contextually specific items when reviewing code. Broad comments lead to a loss of context. If broader discussions must happen, they should happen outside of code reviews. This keeps the code review clear and scoped to the code that is being reviewed.
So, as the conclusion, this list includes general, high-level things to consider for those who are making code review and whose code is being reviewed. Be positive, be open, and less worry about being hurt or hurting others. Good luck!