Share this article

Share on facebook
Share on twitter
Share on linkedin
Share on reddit

The difference between good code reviews and great ones

This post lists the key insights from this article, from an Uber engineering leader, akumni of Skyscanner, Skype & Microsoft.


Areas Covered by the Code Review

Good code reviews:

  •  They look at the change itself and how it fits into the codebase. They will look through the clarity of the title and description and “why” of the change.
  • They cover the correctness of the code, test coverage, functionality changes and confirm following the coding guides and best practices.
  • They will point out obvious improvements, such as hard to understand code, unclear names, commented out code, untested code or unhandled edge cases.
  • They will also note when too many changes are crammed in one review, suggesting keeping code changes single purposed and breaking the change into more focused parts.

Great code reviews:

  •  They look at the change in the context of the larger system, as well as check that changes are easy to maintain.
  • They might ask questions about the necessity of the change or how it impacts other parts of the system.
  • They look at abstractions introduced and how these fit into the existing software architecture.
  • They note maintainability observations, such as complex logic that could be simplified, test structure, duplication and other possible improvements.

Tone of the Review

Good code reviews:

  • They ask open-ended questions over making strong or opinionated statements.
  • They offer alternatives and possible workarounds that might work better for the situation.
  • They assume the reviewer might be missing something and ask for clarification instead of correction, at first.

Great code reviews:

  • They are also empathetic. They know that the person writing the code spent a lot of time and effort on this change.
  • These code reviews are kind and unassuming.
  • They applaud nice solutions and are all-round positive.

Approving vs Requesting Changes

Good code reviews:

  • They don’t approve changes while there are open-ended questions. However, they make it clear which questions or comments are non-blocking or unimportant, often called “nitpicks” or marked with “nit”.
  • They are explicit when approving a change – e.g. giving it a LGTM.
  • They are equally explicit when they are requesting a follow-up, using the code review tool or team convention to communicate this.

Great code reviews:

  • They still don’t approve changes until there are important questions that need to be answered or addressed.  
  • These reviews are firm on the principle but flexible on the practice: sometimes, certain comments are addressed by the author with a separate, follow-up code change.
  • For changes that are more urgent than others, reviewers try to make themselves available for quicker reviews.

From Code Reviews to Talking to Each Other

Good code reviews:

  • They leave as many comments and questions as are needed. If the revision does not address all of them, they will note those as well.
  • When the conversation gets into a long back-and-forth, reviewers will try to switch to talking to the author in-person over burning more time using the code review tool.

Great code reviews:

  • They will proactively reach out to the person making the change after they do a first pass on the code and have lots of comments and questions. These people have learned that they save a lot of time, misunderstandings and hard feelings this way, over going back-and-forth on the new revisions.
  • The fact that there are many comments on the code indicates that there is likely some misunderstanding on either side. These kinds of misunderstandings are easier identified and resolved by talking things through.

Nitpicks (unimportant details)

Good code reviews:

  • They usually don’t have too many nitpicks. Too many nitpicks can become frustrating and take the attention away from the more important parts of the review.

Great code reviews:

  • They realize that too many nitpicks are a smell of lack of tooling or a lack of standards. Reviewers who come across these frequently will look at solving this problem outside the code review process. For example, most of the common nitpick comments can be solved via automated linking. Those that cannot, can usually be resolved by the team agreeing to certain standards, following them – perhaps even automating them, eventually.

Code reviews for new joiners

Good code reviews:

  • They use the same quality bar and approach for everyone, regardless of their job title, level or when they joined the company.
  • They have a kind tone, request changes where needed and will reach out to talk to reviewers, when they have many comments.

Great code reviews:

  • They pay additional attention to making the first few reviews for new joiners a great experience. Reviewers are empathetic that the recent joiner might not be aware of all the coding guidelines and might be unfamiliar with parts of the code.
  • They put additional effort into explaining alternative approaches, pointing to guides.
  • They are also very positive in tone, celebrating the first few changes to the codebase that the author is suggesting.

Cross-office, Cross-timezone Reviews

Good code reviews:

  • They account for the timezone difference when they can. Reviewers aim to review the code in the overlapping time between offices, assuming there are a few hours.
  • For reviews with many comments, reviewers will offer to chat directly or do a video call to talk through changes.

Great code reviews:

  • They notice when code reviews repeatedly run into timezone issues and look for a systemic solution, outside the code review framework.

Organizational Support

Organizations with good code reviews:

  • They ensure that all engineers take part in the code review process – even those that might be working on solo projects.
  • They encourage raising the quality bar.
  • Teams facilitate healthy discussions at the on code review approaches both at the team and org level.
  • These companies often have code review guides for larger codebases that engineers initiated and wrote.
  • Organizations like this recognize that code reviews take up a good chunk of engineers’ time.

Organizations with great code reviews:

  • They have hard rules around no code making it to production without a code review – just as business logic changes don’t make it to production without automated tests.
  • They have learned that the cost of cutting corners is not worth it: instead, they have processes for expedited reviews for urgent cases at the team and org level.
  • They invest in developer productivity, including more efficient code reviews and tooling improvements.
5 min​ read

Subscribe to our newsletter Weekly Bytes to get our curation of the top 5 articles of the week.

Subscribe to our newsletter Weekly Bytes to get our curation of the top 5 articles of the week.

Comments