Principles for Code Reviews

Sep 2nd, 2017 in  by Michael Cho

Some thoughts on what code reviews are and are not.

Code reviews are a super helpful process for ensuring code quality, and (IMHO) necessary as soon as the number of engineers working on your product exceeds 3 or so. At its most basic, it is a procedure for validating the design and implementation of features, while maintaining consistency across team members

As I have been spending large chunks of my working day doing code reviews, I've slowly developed this set of principles which may be useful to others.

1. Fresh eyes

Having another engineer look over your work can uncover errors or different approaches to problems. Nobody is perfect, and nobody will always get the "right" solution the first time - especially if another engineer is more familiar with a section of code or the domain problem. 

2. All engineers

By extension, this means there's value for any engineer to perform code reviews at least some of the time. That's right, even the newest or most junior engineers - they very well could be either most familiar or experienced with the issue. It's not necessarily always only the senior engineers who perform code reviews, and it's not always requested from junior to senior.

3. Value both ways

Code reviews are an opportunity for the reviewer to learn too. At the very least they keep in touch with how the codebase is evolving as other engineers work on it, and fairly often they can be exposed to newer approaches too.

4. Not foolproof

It's important to remember that code reviews will not, and are not intended to, catch all errors in your code. There should be other processes in place too - automated tests (unit and functional), sandboxing / manual QA, staging environments, exception handling / alerts, devops monitoring, and so on.

5. Building the "right" features in the "right" way

Code reviews should check both product requirements (to a slight extent) and that the implementation meets these requirements in a manner aligned with other engineers and the existing codebase. 

6. Concentrated reading, not QA testing

This means I am mentally stepping through your code, but I am not (always) actually setting up test users and manually running through your code as a user. Concentrated reading implies thoughtful analysis rather than scrolling through changes - on average I have found I spend about an hour in code review for every day of developer output.

 

You can read on about what to actually look for in a code review here.


Other articles you may like

Prioritized Code Review Checklist - what to look for first, second, and last
Sep 21st, 2017
Running Metabase locally as a service
May 29th, 2017
Create a custom Alexa Skill with AWS Lambda - Part 3 (Lambda)
Dec 18th, 2016
Create a custom Alexa Skill with AWS Lambda - Part 2 (Alexa Skill)
Nov 10th, 2016
Create a custom Alexa Skill with AWS Lambda - Part 1 (Overview)
Nov 1st, 2016