Code Reviews

Phil Tobias
4 min readJun 16, 2021

A code review is such an important step and touches on so many facets of a team. It helps drive culture, collaboration, and mentorship. As your team grows, there is typically no shortage of code reviews for fixing tech debt, new features, security patches, optimizations, etc. Your team might have documentation on how to create a pull request (PR) for review, but that is only the beginning of the workflow. Providing useful feedback as a reviewer can make a huge impact on your team

Code changes shouldn’t be a surprise. Since your team is churning through so many code reviews they should be quick. It can be difficult to keep up with all pull requests. Try to be aware of the changes that are happening even if you aren’t reviewing them. As the creator of a pull request, make sure it is digestible and solves one focused problem. Avoid lumping numerous changes in one pull request. That makes it challenging for the reviewer to process all that is happening. Additionally, it makes it easier to rollback a specific commit if an incident happens. Keep changes small and limit the scope. As a reviewer, you should be able to comprehend what the PR is solving and be able to communicate that back to a peer.

Hopefully, your team has a review process that is mostly automated. Automation will save your team significant time in the long run. Pull requests are a necessity that causes friction. This is good friction and verification from another teammate. The manual review process can take up time and is not something that can be solved by a computer. Reviews are incredibly important, but it is ideal to limit areas a reviewer needs to check for. Leverage linting, tests, and custom scripts to catch all tedious issues. If there is something as a reviewer that you are checking for multiple times in your code reviews then find a way to automate it. If you are doing it, others are probably doing it as well in their reviews. We don’t want to rely on our brain to remember all that we should look for in a review. We might forget or miss something. Keep the cognitive load to doing a review to a minimum. The majority of the reviewer’s job in a code review is a sanity check.

For code that seems off or confusing ask the contributor for more information on this decision. Encourage documentation and code comments so when someone looks at this section again they don’t need to remember why it is this way. Again, don’t rely on yourself or your teammates’ memory. As an engineer, one of the best things you can do is document a feature. This can be as simple as adding a link to a requirements document or a ticket to provide more context for the future. Another option is describing the steps or flow that happen in a file to help others grasp an abstract concept. Your team will become stronger the more you can communicate. You want others on your team to grow in their careers making the team more effective. Lead by example even in your code reviews.

If you are requesting changes, provide actionable feedback. Let the contributor know why you are blocking approval and how they can go about correcting it. This can be as simple as providing an example in the form of a code block. It doesn’t need to be the exact solution. Pseudocode to help guide them to a solution is effective and gives them a feeling of ownership of solving the problem. As a reviewer, you can also provide an annotated screenshot with an arrow focusing on the problem. Make it as clear as possible for the contributor to understand your concerns and expectations. Lastly, it is acceptable if you think something seems off and you don’t have a recommendation. In this case, mention another teammate that might be a better expert or set up a brief call with a few teammates to get feedback so the contributor can move forward.

It should be noted that not every requested change will be blocking. Clarify in your code review comments, which items are blockers and non-blockers. Most non-blockers are probably quick to resolve. Though, there is a balance. If the change is minor and doesn’t impact the functionality or business, it is most likely non-blocking. Don’t add more friction to a review debating over minor details. Hopefully, your automated tooling limits these types of conversations. If you are working on a product there will almost always be tech debt. Track these non-blocking issues to be addressed another time. Of course, we want to limit tech debt, but do accept it is expected and no codebase is flawless.

Lastly, don’t forget to show empathy and appreciation in code reviews. Sometimes there is that file in the codebase that everyone tries to avoid. It is messy or is using a legacy framework. Acknowledge that the contributor stepped up and made that un-sexy code slightly better. That wasn’t an easy task and is often avoided. Thank them for taking that challenge head-on. Imagine if you had done this PR, how would you like it received? This is a win. Celebrate it!

--

--