Nathan Mische asked a really good set of questions in response to my code review presentation posting. I figured that I would give a longer answer than really works in a comment.
First it’s important to point out that formal code reviews are part of a suite of quality assurance techniques. They do not preclude the use of any other, including application design, informal code reviews, testing, etc.
But formal code reviews, a whole bunch of people looking at all of the code, and then having a long meeting discussing it, are relatively expensive. They take a good deal of time from many people who are probably working on their own code, and incurring the cost of context switching by doing so. So my guideline for code reviews is once per major revision of the application. Basically, if you spend a few days doing an update on an application that has already been reviewed, then you probably don’t need a formal review. If you and a team of developers spend 4 months doing a revision to the code, a formal review is probably in order. Those are the extreme cases; your organization will have to choose where in between to draw the hard fast line.
As for when to do them… We at Wharton do them basically right before the application moves from development to production. This is also the time when we try the application out on the staging server. The timing is dictated by how we’ve mandated code reviews. To get your application on our production ColdFusion servers you must go through a code review.
Is that the best time? I’m not really sure. I think if you design correctly, unit test, informally review code, and get user feedback from your clients, then it is the right time to do the review. But we don’t always do all of that so I get the subtle feeling that earlier might be better, but we’ll just have to deal with the timing forced by our mandate.
Anyone else have any opinions on this?
Like so many things, it depends on the situation. If you have new staff or have just implemented new standards, more frequent reviews help transfer knowledge and ensure that the team is on the same page. Since they are time-intensive, we have at most one a week.
LikeLike