My employer, Indeed.com, hosts engineering discussion panels and recently I have been a part of our code review discussions. Participating in these panels has forced me think intentionally about how I approach code reviews. This is an attempt to synthesize my approach.

Overview

I learn most efficiently when giving and receiving reviews and it seems to stick better than just reading or hearing advice passively. I see them as a way for me to help better others, help better myself, and help better the software that I build.

I have also learned that code reviews are should not be primarily used to catch bugs, that’s just a side effect. Code reviews are effective as a learning and teaching tool. They help elevate the ability of the people you build software with. The constant feedback cycle of doing and receiving code reviews is a useful tool for increasing software quality and readability. When doing code reviews you know someone else needs to understand your code and this helps emphasize clear and expressive code.

Receiving Reviews

Review your code first

Before asking someone else to do a code review, review it yourself. I usually do this in the actual CR tool. This is an easy way to catch small things that you miss when heads down coding.

Assign someone specific

If there isn’t a specific owner then often times code reviews get delayed. It’s not that others shouldn’t look at it, but you want someone to be responsible for it. This prevents the bystander effect.

Be kind to your reviewer

Give as much context as possible

Usually team members are familiar enough with your work that they have context already, but you can’t make that assumption. The intent of the change should be clear. This is easily done by making sure the description in the CR is accurate and updated. If there are diagrams or design docs related to the change, link them in the CR. Make sure the description in the issue tracker is up to date and link that in the CR.

Try to keep reviews small

This is one of the hardest things for me, but try to break changes into digestible pieces. If I see a review that is thousands of lines long and touches tens of files I dread it. I try to be conscious of that feeling when asking others to review my code. If a change does become large, break it up into smaller components and review each of those individually.

Doing Reviews

Be Quick

Often times when someone asks you for a code review they become blocked on you completing the review to finish work. There is usually a tree of dependencies waiting for each review and getting them done quickly helps the entire team.

I recommend setting aside time for code reviews in 2 ways:

  1. Check for assigned code reviews before standup.
  2. Check for assigned code reviews after submitting one myself.

When I do see a code review that I am responsible for I ensure that I look at it before I start any new tasks with one exception: large reviews. If a code review is large and will take longer than 15 - 20 minutes then block out time to look at it later that day.

With this approach I usually have a 24hr or less turn around for code reviews. This also give me something productive to do between waiting for my own reviews or starting a new task.

Gain Context

When reviewing code you are likely to have some context around the change already. If you do not have context already be sure you understand what the change does and why. The goal here is to ensure that you know what you are reviewing and the scope of the change.

If you were involved in the design or conception of a particular feature this step is fast. I start by looking at the ticket or merge request description to get the big picture. Next I will look at the commit messages. These are often smaller than a change description, but helps to understand the scope of a change. If I still don’t have enough context then I reach out to the requester and ask for clarification.

### Automate the boring things A huge time sink in code reviews is feedback that should be caught with a linter or formatter. It’s common to see comments like “This variable should be final”. This is low hanging fruit and should be caught before a code review starts. Teams should standardize on a style guide and use common linters to analyze code before it is reviewed. If there is something in a code review that seems like it should be caught with a linter then you should update the linter. This prevents noise in a code review that can cost time and increase cognitive load.

### Start with the tests Once I understand the scope and intent of a change I start by looking at the tests. This usually gives me examples of typical call patterns and implementation expectations. I can also usually tease out assumptions and identify system boundaries based on mocked classes.

Watch the boundaries

When looking at a CR I ask myself two questions:

  1. Are we changing data coming into the system?
  2. Are we changing data going out of the system?

If the answer to any of these are yes I tend to look at things on these boundaries more intently. A short list of things I think about at the boundaries of a system include: 1. Backwards compatibility 2. API design 3. Dependency coupling.

There is a lot of opportunity at boundaries to introduce errors and changes at boundaries can have a large blast radius.

Darin’s Law: Cover your A.S.S.

When dealing with large scale systems there is a lot that can go wrong. At Amazon I worked with a principal engineer who distilled this down to 3 topics: Availability, Security, and Support. While doing code reviews I always keep those things in mind.

To ensure you are covering your A.S.S it use useful to ask your self:

  1. Can this change cause failure in your service or a dependency?
  2. Is this change doing anything that can impact customer data?
  3. Will I be able to diagnose issues with this change later?
  4. Will I be able to easily update this code path later?

Summary

Code review is a useful tool for helping improve both software quality and technical acumen. The code review process is fun and has been a key component of my growth as an engineer. When requesting code reviews remember to be kind to your reviewer, you want to make the process of getting feedback as painless as possible. While reviewing code pay attention to your boundaries, and don’t forget: Cover your A.S.S. in code reviews.