05. Practice

Code review before there were tools for it

The finding that survived every change of process is a reading rate, and it is the one most often ignored.

For a separate operational view of time, ownership and team activity, see the Monitask overview.

code-review-before-tools.src
royce1970.pdf fig. 2 — "this concept is risky"
# the diagram everyone copied

1976The paper that formalised it

Reading each other's work predates any process, but the formal version was described in a manufacturer's technical journal in the middle nineteen-seventies and it is specific in a way modern practice is not.

Defined roles: somebody chairs, somebody reads the material aloud in their own words, somebody looks for defects, and the author answers questions and does not defend. Preparation alone beforehand is mandatory. Defects are recorded and counted rather than discussed.

The rule that mattered most

A reading rate. The published guidance is on the order of a hundred and fifty lines an hour, and reviews conducted faster were found not to find things.

That single number has survived every change of tooling and process since, and it is the finding most often ignored. A review of eight hundred lines completed in twenty minutes has not happened, whatever the record says.

1970s and 1980sWhat it was measured to achieve

The original reports claim removal of a substantial majority of defects present before testing began, and later studies in various organisations reported similar magnitudes.

Those figures come from the organisations running the process, counting defects by their own definitions, mostly in large regulated development. They agree with each other broadly and none of them is an independent measurement.

Reviews find different things than tests

Worth stating because it is the durable argument for doing both. Testing finds behaviour that differs from expectation. Reading finds code that is correct for the case tested and wrong for a case nobody wrote a test for, and it finds things that are not defects at all: unclear naming, duplicated logic, an assumption nobody wrote down.

A test cannot report that something will be hard to change next year.

1990sThe part that was dropped

The meeting. Studies in the early nineties compared inspections with and without the group session and found the meeting added little to what individual preparation had already produced, while consuming the most expensive resource in the process.

Which is what modern practice does: everyone prepares alone, comments are written down, and no room is booked. The process arrived at that arrangement partly from evidence and mostly because distributed teams could not have the meeting anyway.

2000s onwardsWhat tooling changed

Review became attached to the unit of change rather than to a document, which the entry on distributed version control explains: once a change is an object with an identity, it can be discussed, revised and approved as one.

That made review continuous and cheap, and it also made it a gate. In most organisations approval is now required to merge, which turns a defect-finding practice into an access control mechanism, and the two purposes pull in different directions.

What modern review is actually for

Honestly: less for finding defects than the formal process was, and more for spreading knowledge, keeping a codebase consistent, and creating a moment at which somebody other than the author has looked.

Those are valuable and they are not what the original measured. Comparing the two on defect removal rates therefore compares a practice against its ancestor on the ancestor's terms, which flatters neither.

What a reviewer should be looking for

The formal process listed defect types explicitly, which modern practice mostly does not, and reviewers therefore look for whatever they happen to notice.

A short list travels well: cases the author did not consider, assumptions not stated anywhere, error paths, and whether the change does what its description says. Everything a machine can check should already have been checked by a machine before a person is asked to look.

present dayThe cost nobody accounts for

Review is now on the critical path of almost every change, which means the delay between requesting and receiving one is a direct component of how long work takes.

In practice that delay is frequently longer than the work itself, and it is invisible in every measurement of developer productivity because it is nobody's activity. It is a queue, and queues are the part of any process least likely to be measured by the people inside it.

The formal process, 1976prepare alonemeetlater found to add littlelog defectsreworkfollow upThe finding that ""survived all of itread slowly, or find nothing. the rate matters more than the ceremony.
FigureThe stages of the formal process, with the meeting marked as the part later found to add little, and the finding that outlived all of it.

Who should not be the reviewer

One structural point the formal process was clear about and modern practice frequently is not: the author does not defend, and the reviewer does not decide whether the author is any good.

Where review is also the mechanism by which people are assessed, the incentive to raise something uncomfortable disappears on both sides, and the practice degrades into approval with commentary. Keeping the two apart is a management decision rather than a tooling one.

present dayThe failure mode everybody recognises

A large change arrives, nobody can hold it in their head, and the review consists of comments on naming and formatting because those are the defects a reader can find without understanding the whole.

The remedy is the reading rate again, applied in reverse: make changes small enough that a reader can go slowly through all of them. Almost every complaint about review quality is a complaint about change size wearing a different hat.

What we cannot verify

The original paper and the studies of meeting effectiveness are published. Defect removal figures are self-reported by the organisations concerned and are not independently reproducible. Claims about the effect of modern review on defect rates come mostly from single-organisation studies whose conditions do not transfer.

In short

  1. The formal process defined roles, mandatory solo preparation and counted defects.
  2. The finding that survived everything is a reading rate of about a hundred and fifty lines an hour.
  3. The reported removal figures are self-reported by the organisations running it.
  4. Reading finds what testing cannot, including things that are not defects.
  5. The meeting was dropped after studies found it added little to solo preparation.
  6. Attaching review to a change made it a gate, which pulls against finding defects.

also in Practice

Next.

further context

For a primary or institutional reference, see the NASA Software Engineering Handbook.

Every claim here carries the source it came from.

The source and its year sit beside the sentence they support. A secondary account is marked as one, and where the record is unclear the entry says so rather than choosing the better story.