Don't companies send developers out for code review training anymore? I had to take a pretty serious course in it in my days as a full-time developer. A couple things I rememeber were.

Someone takes good notes and it is NOT the person whose code is being reviewed. Someone else reads the code and moderates the discussion. The person whose code it is should be answering questions not running the review. Also the whole process has to be non judgemental. If someone starts beating up on the programmer the moderator needs to step in and stop it. The experience should be a learning one not a painful one. Well not any more painful then required.

Reviewers should read through the code before the meeting. They can bring questions if they want but not ask them until the meeting gets to that part of the code.

If there is a manager in the meeting they should be there because of technical expertice not to evaluate any of the individuals in the meeting.

I'm quit interested in this as well. I've been tasked with getting code reviews going on my current project and I have no clue how to do so without coming off as a code nazi.

I have found this book is good (and free) - www.smartbear.com/codecollab-code-review-book.php" rel="nofollow">www.smartbear.com/codecollab-code-review-book.php">www.smartbear.com/codecollab-code-review-book.php" rel="nofollow">www.smartbear.com/codecollab-code-review-book.php. Don't know anything about the software from the company but the book is really only pushing code reviews not their software.

We've only just started doing code reviews but so far they've been working well. No confrontations yet but not everyone on the team has been through the process yet!

One minor addition I did add, was to link the need for a code review to a basic points system. Adding points for complex code and core libraries, subtracting points for pair programming - once it hits a threshold, a code review is triggered.

Hi Jason,
I put together a few posts on my blog a year ago on this kind of stuff. blog.philknows.net/CategoryView,category,Code" rel="nofollow">blog.philknows.net/CategoryView,category,Code%2BReviews.aspx">blog.philknows.net/CategoryView,category,Code" rel="nofollow">blog.philknows.net/CategoryView,category,Code%2BReviews.aspx

Phil

What I've found when trying to come up with a code review process is that there are a lot of different things that people try to accomplish in a code review that may not actually match what your definition of "code review" is. Might be good to level-set on what everyone wants out of the process before defining it. Some of the things I've found people want:

* Design review
* Architectural review (in context of the code that was written)
* Education for junior developers
* XML API documentation review
* Unit test review
* Integration test review
* Interface/contract review
* Detail-oriented line-by-line review of code (make sure i's are dotted, t's are crossed)

Some of these, to me, belong in code review, and some don't. Some people also think a code review should be something that happens before code gets committed to the main production codeline, some people think it can happen after that - scheduled as a separate task with a separate priority.

Oh, and think about some of the "non-functional requirements," too. Are you working with a distributed team? Getting everyone in a room, even a virtual room, all at the same time may not be feasible.

In the past I've used the Smart Bear "Code Collaborator" product and like it. It addresses the points that I feel should be in a code review and answers a lot of the non-functional things like scheduling and team distribution. The new 4.x version of the product is superb for working with Subversion, too.

Code reviews usually don't happen unless a scheduled block of time is planned in advance for the meeting(s) (hopefully at the start of the project), all participants are available to attend, reviewers are given time to review the code (each reviewer is assigned to look for a specific problem to cover different scenarios) etc. So it's a big scheduling problem plus the problem when the developer under the microscope gets defensive. You definitely need an experienced moderator.

I'd stick to unit tests and let everyone else learn how to be an architect on their own time.