Monday, February 11, 2013

Problems with code review are symptoms of problems in your team or engineering

The only process Google forces on teams is code review. Nobody is exempt, though teams may have varying strictness in their review.

We do code review at ZenRobotics too. It mostly works, but maybe not extremely well.

I see code review (in general, not just at ZenRobotics) not working as a symptom: it indicates a problem elsewhere. I've seen at least the following underlying issues getting reflected in code review:

  1. Lack of tests: if you don't have good tests, you are very reluctant to make changes in code that you think works. This means you resist the changes that others propose, since you fear you'll break your code.
  2. Code that is difficult to understand (including convoluted designs or architecture): if the code is hard to understand, people will either require a lot of comments (which may be seen as counterproductive), extensive rewriting (without specific guidance on how to re-write since they didn't understand what your purpose was), or less-than-useful reviews that just look at the surface.
  3. Style guide violations: both reviewers and people who wrote the code under review quickly get annoyed at stylistic comments. At Google the idea was that  you internalize the style guide under the first couple of weeks of using a language and after that you don't need reminding. Code in review with style violations may also mean you lack tool support to make style checks easy. It could also mean that your team-mates don't actually agree on your coding style, which makes it pretty useless.
  4. Not knowing the components others are working on / not having time to understand them when reviewing: if you don't have at least one other member on your team who understands the stuff you work on, you have a very low bus factor - could even mean people are starting to create fiefdoms, or that code reuse is very low.
Now fixing issue no 1 (Lack of tests) or 3 (Style guide violations) doesn't make reviews free. However a lot of the cost of reviews is in fixing 2 and 4 - which you want to fix anyway. Especially, code reviews are great for implementing shared ownership and knowledge transfer.

(This doesn't mean I'm certain that code reviews suit every situation. What if you want to quickly iterate on your MVP? What if you want to quickly prototype an idea or a design? Anybody else have an idea?)

No comments: