   How to conduct peer code reviews
  At our company we have a code base that is shared amongst 5-6 different applications.  We're about to implement a peer code review process for all checkin's to the code base.I'd be interested in knowing how you think we should be handling the code reviews.  What I'd like is a way to view another users change on my machine so I can diff it to the head of our cvs tree to view the changes and build it.Any option is on teh table at the moment.  Ones we're considering are - each change is a patch- each change is on its own seperate branch - each change is zipped up and emailed to the reviewer.Perhaps there is a spiffy tool I don't know about that does this??  What do you people use??CheersChris   
  We do peer code reviews by walking over to the person's desk, and reading over the code, asking questions, and looking at diffs vs CVS.;  Thanks:)I'd rather avoid that option as we'd ideally like people to review the changes at their own pace/time without making the person who wrote the code wait:)CheersChris   ;  We use perforce, and have a web interface to look at any client and see their changes. But for the larger code changes, it's a lot better to go over to their desk and have them talk us through it. Why? Have you ever stared at code really long and still been unable to find that weird bug? And then, you talk someone through your code, and while you're talking, you kinda say "oh never mind, I found what was wrong..." It forces the coder to explain specific design decisions and such things, which aren't in their code.   ;  I like to have the developer lead an in-person walkthrough as well.  It helps solidify their understanding and lets the reviewing team understand that developer's intentions behind the new features as well.  I would question what activity you are trying to optimize by allowing your developer to keep charging ahead before their code is reviewed.  You can use CVS branches, but you are likely to spend a lot of time merging and re-synching - I know I would get annoyed at updating sticky tags all the time.  Programmers need to talk more! ;)   ;Quote:Original post by chollida1I'd rather avoid that option as we'd ideally like people to review the changes at their own pace/time without making the person who wrote the code wait.The peer review should be done before the code is checked in. So the author will have to wait regardless.The author and the reviewer should go over the changes together. The reviewer needs to be able to ask questions. The author needs to explain stuff.;  I'll have to second JohnBoltonIn my previous job we had a 'buddy' system, where no code was checked in to source control without the changes first being reviewed by a 'buddy' (usually just whoever's available, unless you're working on something that requires in-depth knowledge of the system). All check-ins were also tagged with the name of your buddy partner, so you're both responsible for the code that goes in.   ;  If these options won't work for you, and you simply want to compare files on a raw level, you can use WinDiff (spelling?).  This will simply look at two files and show the differences between them.Of course another option (or probably something you should do in conjunction with the above suggestion) is to have each programmer add a TODO: comment block before any new code, then you can just search through their files for the TODO: and see their notes about what they did.  We did this extensively, giving a brief description of the task, our initials (accountability is good), and a date.  That way, whenever we integrated we just searched for these comments, and applied changes to our code bit by bit.  If something breaks in that particular module, we know what probably broke it, and if need be, we know who exactly to contact about what they were doing in any given code segment.This may be more of a taxing process then you're looking for, but it certainly worked for us.   ;  As a side note, one of the things that is also good to do, and saves time for the reviewer is to make sure that code is up to standard before being reviewed.For example I had to review a workmates VB code and I knew it was going to be sub-standard from what I had seen, so I told him to download and install Project Analyser, and fix any errors that it tells him about, that might be worth fixing.btw I highly recommend that tool.I also recommend getting both the review and reviewee together for the reviews, but if that isn't an option, then make sure that a reasonable amount of supplementary information accompanies the code. Either in the form of design docs, plenty of code comments, or even just a scrap text file with notes about why some bits are done the way they are, and the problems they had to overcome etc.   ;  We use the Code Reviewer by Smart Bear Software (www.smartbearsoftware.com).  It extracts source code diffs and packages them for other reviewers in a shared network area.  We use it with Perforce, and it also works with CVS and other source control systems, although the integration with CVS is different from Perforce.  They also have a new peer review tool with more features that we haven't evaluated yet.   