Every formal code review I ever sat through was a painful experience that went on for longer than planned, covered way less code than planned, resulted in minor improvements at best and had zero long-term impact on the quality of the codebase.
Of the many reasons for this, I'll mention one: by the time a code review takes place, the key design decisions surrounding that part of the system have typically already been made. To do a retroactive redesign during a meeting is out of the question; that would be more work than writing the original code in the first place. So all people end up doing is scrutinizing tiny things like null checks. I'm not saying those don't matter; of course they do, but they're only a small part of making good code.
So the code-review approach boils down to saying "do the big stuff on your own and look over the small stuff together". That seems backwards to me.
(By the way, I'm talking about commercial projects. There's an entire literature around formal inspections, as you know, and good results have been said to be obtained with them, but if you want to go that route you're talking about heavy-duty process control, slowing development to a glacial pace that is unworkable for most commercial software.)
I've personally observed two things that do work. First, pair programming. It's not for everybody (the community here, for example, is largely derisive of it), and I'm not arguing for it in general. But there's no question that it does what code reviews are supposed to (but don't).
The other thing I've observed to work is to write the code individually but review it on checkin. That's how we do it on my current project. We'll either watch the repository and look over changes as they are committed, or email a patch and ask for review. This kind of review isn't as efficient or as comprehensive as writing the code together in the first place, but it does allow major problems to be caught and dealt with quickly (at the cost of some rework).
Of the many reasons for this, I'll mention one: by the time a code review takes place, the key design decisions surrounding that part of the system have typically already been made. To do a retroactive redesign during a meeting is out of the question; that would be more work than writing the original code in the first place. So all people end up doing is scrutinizing tiny things like null checks. I'm not saying those don't matter; of course they do, but they're only a small part of making good code.
So the code-review approach boils down to saying "do the big stuff on your own and look over the small stuff together". That seems backwards to me.
(By the way, I'm talking about commercial projects. There's an entire literature around formal inspections, as you know, and good results have been said to be obtained with them, but if you want to go that route you're talking about heavy-duty process control, slowing development to a glacial pace that is unworkable for most commercial software.)
I've personally observed two things that do work. First, pair programming. It's not for everybody (the community here, for example, is largely derisive of it), and I'm not arguing for it in general. But there's no question that it does what code reviews are supposed to (but don't).
The other thing I've observed to work is to write the code individually but review it on checkin. That's how we do it on my current project. We'll either watch the repository and look over changes as they are committed, or email a patch and ask for review. This kind of review isn't as efficient or as comprehensive as writing the code together in the first place, but it does allow major problems to be caught and dealt with quickly (at the cost of some rework).