Better Code Reviews
Become a more effective and empathetic reviewer
Reviewing code is an daily occurrence for developers. It can be a humbling learning experience, but it can also turn into an egocentric process. Over the last few years I’ve learned some best practices, either through personal experience or excellent advice from peers, that have helped me to become a more effective and empathetic reviewer. I wanted to share some of this information, in the hopes that it will help others. This short post is broken into two main parts: 1. getting the best feedback on your work and 2. providing great reviews for your peers.
Structuring Your PR’s to Make Their Life Easy
Avoid massive changelogs
- Keep it small — under 300 lines of changes, if possible.
- If a piece of work requires more than 600 lines try to break it into smaller logical chunks that can be reviewed individually.
- No one wants to be sent a 1000 line PR monstrosity. Good reviewers will often reject a pull request of this scale; they know from experience that they probably won’t be able to to review it effectively.
Structure for comprehension
- Group similar changes into commits. Changing the location of a bunch of files without modifying them? Group that into its own commit, so the reviewer knows it’s not a logical change they need to focus on.
- Commit changes frequently to reduce their scope. For larger pieces of work it is far easier for your reviewer to focus on what’s important when the commits are small.
Short-circuit possible confusion
- If you modify logic or structure that existed in a previous set of commits, ensure you inform the reviewer so they don’t get sidetracked into critiquing now non-existent code. This is applicable for when the reviewer is going through commit by commit.
Provide enough context
- Ensure that your PR is described clearly, whether this is through a linked ticket or a written description in the PR, or preferably both. Provide examples, visual or otherwise, of how your work affects the project. Look at things from the reviewer’s point of view and try to make it as seamless as possible for them to understand the purpose of your pull request.
Approaching a Teammate’s PR Efficiently and Effectively
Review in a timely manner
- Once you are added as a reviewer on a PR, try to review it as soon as possible.
- The maximum time from a request and the first review being completed should never exceed one work day.
Gather context if it’s lacking
- If the PR is lacking context, or a description of its changes, reach out to the writer and get clarification before reviewing. This can significantly reduce back and forth caused by misunderstandings of the code or purpose.
Start at a high level
- Review from the high level down. Hold off on commenting on small changes like style until the larger issues, if they are present, are addressed. This way the author can focus on the most important revisions first. Sometimes the smaller issues will go away along with the larger changes.
- This also cuts down on the possibility of bike-shedding, which detracts from the more important and impactful aspects of the work.
- If you disagree with a code block, provide a code example of how you would suggest it be done instead. This is far more useful to the other developer than just leaving a the comment, “this should be different”.
- Don’t block a PR unless there are fundamental issues with the work. Putting a roadblock in front of your coworker over an argument around naming doesn’t help anyone on the team. Give your teammate the opportunity to address the smaller changes and move on with their tasks without waiting for another round of feedback.
- Small issues should be addressed through a project style guide — but that’s a different topic altogether.
- Most importantly, remember there’s another person on the end of your review. Provide feedback in a helpful and constructive way. Always think about how you would want to be given feedback, if it was your pull request that was being reviewed.
These are some of the pieces of advice I’ve gained from my peers and from personal experience over the years. Each has been directly applicable to me at one point or another, and I hope that they will each help you out as well.