Code review culture
I worked in different kinds of companies. Somewhere reviews were the essential part of work and the only way to grow junior developers. Somewhere I was hired as an expert who were not supposed to ask for reviews. At Shopify the level of code reviews goes even higher. Even when I thought that I got the “good enough” reviewing skills, I had a lot to catch up. This post is about what I’ve recently learned about the reviews.
Think beyond the code
Before commenting about the code style, think about the design. Would the approach work at all? Would it scale? How do you test and verify the results? Did the code author consider alternative implementations?
Discussing it prior to the implementation saves time both for reviewer and the author. It’s painful to make the no-go decision after the code has been reviewed and polished.
Actionable reviews
People expect you to find errors that they didn’t spot. Don’t just read the code — try running it. Maybe the platform-dependent tests would pass on author’s Linux machine and would fail on your macOS laptop. Maybe there’s an issue that can’t be spotted by looking at the pull request diffs and you would only find it see the full context.
Try helping rather than asking questions.
Double check the libraries.
Watch the language
This was one of the hardest things for me. As most of Russians and Europeans, I come from the background where it’s totally fine to express the feedback in a very direct manner. But what is acceptable for us may be offensive for people from other backgrounds. Don’t talk too direct and be friendly.
Wrong comment:
Change this line to X
Better comment:
You may want to use method X here because it would catch unexpected results
Wrong comment:
Move this into method A
Better commit:
I feel these checks could go in the method A. What do you think?
Wrong comment:
Use A to generate B, in this case you won’t need to mutate the array.
Better comment:
I think we could use A to generate B and this way we could avoid mutating the array.
Don’t take strong opinions. Try to be open to other solution than what you suggested.
Cheer up
It’s not enough to just leave the comments about code. Express your thoughts about the proposed changes.
Great work! Feel free to merge when the comments are addressed.
I’m excited to see the feature landing into the product. LGTM except minor nitpicks.
Think from the outside
Try to read the code as you’d have no context in that area. Look on the code as you’ve never seen it before. Would it be readable for a new team member? Moving code around to increase readability always helps.
Acknowledge the review request
If you can’t review it right now, go and comment about it. You’ll help another person to plan their time if they now when they can get the review.
Healthy code review culture does incredible things to your team. Stop doing reviews and you’ll see your colleagues stalled in professional development. Level up the reviews and you’ll see how quickly people can improve. From my experience, most efficient and professional teams always have a high bar for the quality of peer reviews.
Big thanks to my colleagues. I’d never learn these things without them. I wish I got this knowledge earlier and I’d be more nice to people.
Further reading: Four rules for arguing intelligently and criticizing with kindness
