CM CodeReview

Code Review

A meeting at which software code is presented to project personnel, managers, users, customers, or other interested parties for comment or approval. Contrast with code audit, code inspection, code walkthrough. See: static analysis.

-- PatrickEgan - 18 Dec 2002

I consider holding code reviews, especially before commit, as a bad advice. There are several reasons for it:

  • Reviews are often meetings (OK, code reviews can be held by mail, or in a forum, but this doesn't really help for the other reasons developed below), and meeting are bad in general.
  • Reviews introduce an artificial state in the workflow, a synchronization point, which is in total contradiction with the continuity agility should strive for.
  • Reviews focus on sources which is wrong, maybe the most harmful Zeitgeist of software development. Software is software because it can be run. The effects of running software are objective in a unique way, which gives testing its (often misunderstood as some kind of validation criterion) value. The changes are better mapped in the domain of functionality, of derived objects: you fix your car in a garage, but it is on the road that you'd like it to work, that you'd like to manage it.
  • Reviews are synchronous and expensive, and cannot happen as often as code should be refactored.
  • Holding a review will delay commit, i.e. publication, which is typically (sadly) coupled with delivery. The need of communications is course the contrary: to publish continuously, in advance, at the stage of intentions, so as to avoid duplication and clashes; and merges which could have been avoided. Most of the time, delivery should be pulled, not pushed. Or at least the balance should be adjustable by the end user, as was available in base ClearCase, with config specs and labels (i.e. before UCM or administrators hijacking them for other purposes).