Informal Reviews

It would be much appreciated if we could keep this page an XpFreeZone. Please use ExtremeReviews or IssuesOnReviews for comments on InformalReviews and XP. (see FormalInformalReviews for meta-discussion)

As an example of how someone else does informal reviews, consider RobertMartin's outfit at ObjectMentor Associates (http://www.oma.com/). Robert is big on small process, and is known to express the sentiment "the best process is the smallest process you can afford" on multiple occasions. Robert says that he wants reviews and other meetings to take up no more than 5% of the engineer's time (I think I recall the number correctly, but my memory may be faulty here).

As for code reviews, Robert says that what he does is decide whether the change is non-trivial enough or else "high stakes" enough to have another pair of eyes glean it. If it is, he hands it to an associate and asks if they could look at it and get back to him with any comments or questions within a few days. That's about as informal as they get, but the main thing is that he still has a "fresh" pair of non-author's eyes give his work the once-over.

I've seen something only slightly more formal than this work well. The code (or artifact) is handed out to at least two non-authors who look it over within a day or two and mark any comments or questions they have. Then they get together for an hour (which means the code can't be too large) and instead of reading line by line, they go page by page asking if there are any comments or questions for each page (or else they ask people what page their "next" comment/question is on, and goto the earliest one, than the next, in page order). Comments and questions may deal with errors, clarifying what is happening, or probing into issues of design or refactoring. So this kind of review is at least as much about sharing understanding of the system as it is about finding errors. Many formal inspection aficionados wouldn't approve of this, but I've seen it work well for many shops and projects.

I've participated and observed many different kinds of reviews for many different kinds of projects (from formal inspections using the methods of Fagan, Ebenau & Strauss, or Gilb & Graham, to informal reviews and walk-throughs (and some of the other types of reviews mentioned in the Freedman & Weinberg book from Dorset House publishers). I've seen instances where each kind of review was highly productive, and where each kind of review was largely ineffectual. The primary invariant I've observed has to do with the professional social relationships between the review participants. If these folks are comfortable with each other and having faults in their efforts identified without raising personal issues of self-esteem or ego-feeding, and are genuinely more interested in fixing and understanding and improving the product more than they are in feeling perfect or expert, then just about any kind of review seems to add value. If not, run for cover quick ;-)

-- BradAppleton

Then they get together for an hour (which means the code can't be too large) and instead of reading line by line, they go page by page...

This sounds to me as if the code changes are already too large. An hour? Most of the code reviews I've done took half an hour or less. Page by page? Most of the code changes I've made after we implemented reviews fit easily on a single page, using uni-diff format. -EdGrimm

Remember that each pair is reviewing the other pair's changes during this hour. It's not just one change - each pair has their turn to be on the giving end as well as the receiving end :-). The "hour" is an approximate maximum for the time it takes to review both sets of changes, often it takes less (it depends on when you run out of donuts ;-). -- BradAppleton

"Informal reviews" have as much effect on proper engineering process as pixie dust on a charging rhinoceros. If you don't apply some formalized inspection mechanism -- complete with consequences -- then you may as well just take a coffee break.

See: FaganDefectFreeProcess