How to do a code review

Review code in this order: protocol buffers, unit tests, headers, implementation. It's common for a new employee to be an expert on C++ or Java or whatever languages but it's very uncommon to meet anyone who knows how to define a decent protocol message. The tests should give the reviewer a very clear idea of what's happening in the code (and this should be congruent with the description of the change), if not send back to author at this point. The headers should contain clear interfaces, types, and comments and should not contain anything that's not part of the API (when this is technically possible). Finally look in the CC file; at this point the reviewer should see things they were already expecting and no funny business.

Any claims about speed, efficiency, or performance whether in comments or during the review must have accompanying microbenchmarks or they should be deleted. Wrong ideas about software performance abound, and even correct beliefs become incorrect with time, in which case the microbenchmarks are critical to evaluating the continued value of the code.

When sending a changelist for review, always clear all automated warnings or errors before wasting the reviewers' time. Nobody wants to see code that doesn't build, breaks a bunch of tests, doesn't lint, etc.

Imagine trying to evaluate the quality of a novel by examining diffs to the manuscript. Reviewing every diff means that every single change needs to yield a good novel. But that's an absurd constraint on the creative process - what if you want to introduce a new important character? You check in a new version of the first chapter where the character figures prominently, but the reviewer complains that the new character's plot arc never goes anywhere.

The main goal of this diff-based code review seems to be to prevent people from checking in bad code. But of course you should sometimes check in bad code (especially in the early stages of a project)! You might consciously check in some bad code temporarily, intending to upgrade the code at some future point. More importantly, you generally don't know how or if you're actually going to end up using any given piece of code. Spending a lot of time polishing and reviewing code that doesn't eventually go into the project is a classic example of premature optimization (the root of all evil).