Doing code reviews is crucial for any successful engineering team, but without being given care it is easy for the process to devolve into one of stress and hurt feelings. As an engineering leader it’s well worth your time to encourage a culture of respect and professionalism towards code reviews. Outlined below is a way to frame this to your engineering team.
Treat each other with respect
Bringing respect to code reviews means being thoughtful and empathetic on both ends, reviewer and reviewee.
- As a reviewee- you should assume good intent on the part of your reviewer. The reviewer wants to help you make the best decisions for our team and our code base. They have taken time away from their own priorities to contribute to the improvement of the team’s systems. Your initial reaction to comments may be “this is a waste of time, this is nitpicky and not a big deal, this is dumb”, etc. but challenge yourself to take a step back and see your reviewer not as an adversary, but as a partner in the effort to make smart engineering decisions. Everyone who is reviewing code has a chance to add context, provide knowledge, and illuminate risks, and should never be immediately disregarded.
- As a reviewer- you should assume the reviewee is not stupid. Sometimes simple mistakes are made, anyone who says they’ve never made a simple mistake is either lying to themselves or has never written any substantial code. You should never treat a mistake with condescension or disdain. Additionally, you should recognize that in this profession there is rarely a single right answer on how to do something. Building systems is a series of tradeoffs – time, complexity, performance, cleanliness, etc. Your immediate reaction to seeing a review may be “that’s bad that’s not how I would have done this”. Challenge yourself to take a step back and consider the tradeoffs the reviewee may have made, and consider if in their shoes you can clearly say that it was the wrong decision.
So what’s this mean?
- As a reviewee-
- Don’t be a jerk – if someone leaves a comment you don’t agree with, consider it an opportunity for a discussion, to learn from and build stronger ties to your peers.
- Don’t just drop someone’s comments. Give the reviewer the respect they deserve and not only think critically about the comments they’ve provided you, but also talk to them and learn why they wrote them. If you’re totally positive this is not a comment that needs to be resolved, reply and clearly explain why. Admit that you could be wrong and welcome a discussion if they want one prior to dropping.
- As a reviewer-
- Don’t be a jerk – If there’s time to write a comment then there’s time to make sure the comment isn’t antagonistic
- Ask probing questions without an ego. Assume the reviewee has thought about the problem at least as much as you have.
- Offer your availability to talk things through. – Don’t just leave some comments and run, treat leaving a comment as an invitation for a discussion. You are not an all-knowing code wizard, and just as a reviewee might make mistakes, you might make mistakes in your review.
Some tactical suggestions
- As a reviewee-
- Provide a good overview that contains context on what the patch is for, and if there is additional context e.g. “this is a quick scrappy MVP that we don’t plan to use long-term“ or “this is the new architecture pattern we are trying out to see how well it works”, etc
- Make your code understandable – many miscommunications happen because it’s not clear what the code is doing, be careful about unnecessary complexity.
- If what you’re doing is so complex that it’s hard to understand if you didn’t write it, then add descriptive, well-written comments to your code
- As a reviewer-
- Don’t leave comments that are simple assertions e.g. “use foo.go() not foo.start()”. Your comments should be teaching opportunities e.g. “Based on what you’re trying to do here, to make foo go, you might want to use foo.go() instead of foo.start(), foo.start() does not do the {go subroutine} which based on the overview it seems you want”
- Point to good examples to help support better understanding.
- Try not to harp too much on code style (whitespace, newlines indentation).
On a final note, building a culture is hard. To get this to stick you should reinforce the message in meetings with your team. Encourage every engineer to call out instances of disrespectful review behavior.