cjlotz:
David
I recently wrote a short article on Code Reviews to try and organize my thoughts on the topic. You might find some useful information/links in there.
Nice article!
When we do formal inspections, we do Gilb-style inspections. They have fewer meetings, so are easier to schedule.
There's no kickoff meeting; the materials for the review (the code to be reviewed, plus any relevant checklists, a form to record the defects you found and the time you spent) are either emailed to the participants or handed out by the person organizing the reveiw.
One meeting is scheduled, the meeting to gather and record the defects found. It's scheduled 2 days ahead, or so, to allow the reviewers time to review the materials. At the meeting the defects found are recorded, with no duplicates, and people are encouraged to find more defects during the meeting. At the end of the meeting an outcome is decided by the reviewers. It's one of 'accept as is' (no defects found), 'revise with no follow-up' (a very small number found), 'revise with follow-up' (more defects found and one of the reviewers volunteers to follow up and check the author's changes once they are completed); and 're-review' where there are many defects and the team feels another review is needed. Thus almost always there isn't a second meeting, and when there is it's another review.
Contrast this with Fagan Inspections where there are 3 meetings per inspection (kickoff, review meeting, and followup meeting). Meetings are often the hardest part to schedule, due to peoples' calendars getting filled up, so if you only have to schedule one, it really helps.
Inspections require the use of checklists. And, at the meeting, the moderator asks each reviewer how much time they spent preparing, so there's a check on the time spent finding defects before the meeting. If it's obviously too short for the material at hand, that reviewer isn't prepared. If there aren't enough people at the meeting who did prepare well enough, the meeting is cancelled and rescheduled. You only have to do this a couple of times and people realize we're serious about how much time you spend preparing, and usually they come prepared.
HP found that 80% of the defects found in reviews wouldn't be found in testing, so also reviews are a good complement to testing. They each find defects, but with only partial overlap in the kind of defects found.
Some of Construx's code checklists are available for free on our website. Here's the page that links to them
http://www.construx.com/Page.aspx?nid=97
and some of our Inspection process materials are also on our website, at this location http://www.construx.com/Page.aspx?nid=101
That said, we usually plan to inspect about 40% of the code, and for the rest do one person reviews, which we call desk checks. We'll inspect all the requirements and designs because historically the biggest payback for reviews comes from the early documents. (anywhere up to 100:1). Code inspection typically gives an ROI of 3 or 4 to 1 (relative to letting the defect get into production).
The time spent really matters. There's lots of data showing that you find more defects if you spend more time preparing. The recommended rate for code is 300 LOC/hr or less.
And I think having the chance to review the material before the meeting, when you can really think about it, check if it's consistent with the design etc. is also important. One of the benefits of inspections is the check on the time spent reviewing. Another is that the moderator can limit or prevent personal comments - all the issues raised should be on the work product being inspected and not on the person who did the work. The moderator also keeps the meeting moving and makes sure it doesn't spend much time discussing each defect; the assumption is the author can fix it by themselves if he/she knows where it is; the fix doesn't need to be discussed in the meeting. Inspection meetings are usually quite short and efficient, with issues being recorded every half minute to 2 minutes.