Code Review: Etiquette Guidelines

This is part two of a two-part series on code review. Part one, When your code becomes our code, is about goals and methods for code review. At the end of this article there is an appendix of useful resources on code review used for both articles.

Photo by Nic McPhee (licensed under CC by SA 2.0)

Code review is a powerful tool in producing quality, working code. It can also be a source of friction between coworkers. These are my own personal guidelines to try to reduce that friction and make code review as efficient as possible. They are not endorsed by my company, LaunchDarkly, although they generated little controversy among my coworkers when I presented them internally. I’m also sure I’ve broken every guideline in this list and no doubt will continue to do so, but I find them to be useful guidelines nonetheless.

Guidelines for code reviewers

1. Be constructive and specific. You may have a general critique but specific suggestions provide a direction for discussion and reduce turnaround time. Include example code if possible.
2. Avoid the imperative mood and be careful with the word “you”. Try not to say “you should write this as…”. Understand that yours are suggestions, that they may be mistaken and that they may not ultimately be honored for some reason or other. Consider using “we” instead of “I” as well, which conveys that you’re each part of the same team.
3. Be sure to read the PR description completely. If you’re using a ticketing system — we use Clubhouse — it can also be helpful to read the ticket, so the PR description should give you such a link. This information will typically provide additional context for changes and save some back-and-forth. [N.B. I still find it useful for PR authors to repeat the ticket info in the PR description because reviewers may skip this step.]
4. Make yourself available. As an equal partner in the code review process — see part one — you need to be committed to getting this code that you started reviewing across the line as quickly as possible.
5. Don’t hesitate to do the code review in person. This is sometimes called an over-the-shoulder code review and can significantly reduce latency and confusion.
6. Make it clear if you’re not the primary reviewer. It may make sense to drop in a comment here and there but don’t leave the code author depending on you to complete the review if you believe someone else should be the primary reviewer.
7. You are doing this review for your coworkers, not the code author. Your responsibility to the code author is to be constructive, responsive, and perhaps even to help the author grow as a member of the team; your primary responsibility is to make sure you and your coworkers continue to work with high-quality, correct code.

Guidelines for code authors

1. Break your changes into the smallest possible changes. Do this before the review. You will get better reviews and your reviewers will thank you for it.
2. Read your code before you ask for review. You may even be able to do this before creating the PR, although for some CI workflows it may be useful to create a PR prior to review. We use PullReminders for PR notifications and by naming our PRs with a prefix such as “WIP”, we can tell automatically added reviewers (from Github CODEREVIEWERS files) to hold off before reviewing a PR.
3. Annotate your code in Github before review. You can reduce latency by saving some back-and-forth with the reviewer. If you feel the need to annotate a line, you may want to consider whether you might need a comment in the code or whether the code could be rewritten to make your intent clearer.
4. Respond to all feedback. If you don’t, the reviewer may think you missed it and will feel the need to follow up on it.
5. Avoid taking offense. We’re in this together. It’s understandable that you will feel an affinity for your code; you’ve spent a lot of time with it. Take a breath and move on.
6. Avoid being quick to disagree. Try to understand the reason for or the intent of a suggestion even if you believe the specific suggestion may not be the correct solution.
7. Don’t hesitate to suggest discussing the code in person. For complex changes, this can be far easier as it can reduce both latency and potential misunderstandings.
8. Feel free to offer to make changes in a subsequent PR. Sometimes you just need to get your changes in and this is can be a reasonable compromise.
9. When asking for clarification, suggest what you think the reviewer might mean. This is good advice for any interaction in life.
10. Embrace the reviewer. They will own this code the same as you when this process is done. If a reviewer appears to suggest that something confusing, it probably is.

Appendix: Other resources

There are lots of articles on the internet about why code review is a good thing and how to do it well. Below are a few that struck me as interesting, even if I didn’t 100% agree with them or they don’t match our own unstated policies:

• FullStory’s Lessons From Google: How Code Reviews Build Company Culture
• Code Review Etiquette from CSS Tricks
• Yelp Code Review Guidelines
• Palantir’s Code Review Best Practices
• Developer Productivity and Code Review

A number of the above articles are from a good list published at github/joho/awesome-code-review.