Working as a reviewer

Owned by Raphael ‘kena’ Poss (Deactivated)
Last updated: May 07, 2021 by Adam Storm

We're all humans

First and foremost, it's important to remember that the code you're reviewing was written by another human.  To that end, you should consider the human side of reviewing code before getting to the technical aspects (a good primer of how to do this can be found here).  At a high level, this includes:

  • Being responsive - In general it's best to try and get to code reviews quickly, as the velocity with which we respond to them directly impacts our productivity as an organization.  If the code review is short (will take you no more than 10 minutes to complete), you should try and get to it within a few business hours of it being sent out.  For longer PRs, it may take 24 business hours to respond.  If you think that a review will take you longer than 24 business hours to complete, you might want to reach out to the author to let them know that your review will take longer than expected.  
  • Be clear with your comments - Try to make your comments as verbose and understandable as possible.  Where possible, provide code samples for how you think the code should be changed to eliminate the guesswork. 
  • Start with the big stuff - If you know a review is going to take multiple passes to complete, start by addressing the major aspects (the approach taken, how that approach was implemented, performance considerations, etc) and work your way down in subsequent passes to smaller style changes.
  • Be careful with tone - Some could see your code review comments as judgement of their abilities as a developer, or worse, as a person.  Be mindful of that and try to frame your comments as suggestions/question as opposed to commands (i.e. "Does it make sense to add another test here to do X?" vs. "We need another test here to do X.").  While the tone is much softer in the former, the intention is still just as clear.
  • Be generous with praise - If you see things over the course of your review that are well executed, feel free to point them out.  It can be very rewarding as a Software Engineer to send your code out and have the reviewer respond with glowing comments.  Always take the opportunity to make someone else's day a little bit brighter!

Additional things to watch out for

  • discuss and iterate on overall design before discussing details in the code
  • if you foresee the review will take multiple phases, announce this to the submitter upfront to set expectations about process
  • review code for style, prefix "nit:" for small style recommendations
  • review the commit message: motivation alongside what was changed
  • check and double-check the release note: user-facing changes must be mentioned, and backward-incompatible change must be highlighted

More on the human side of things

Intro:

More detailed guidelines / background:

Science:

Copyright (C) Cockroach Labs.
Attention: This documentation is provided on an "as is" basis, without warranties or conditions of any kind, either express or implied, including, without limitation, any warranties or conditions of title, non-infringement, merchantability, or fitness for a particular purpose.