
It has been over three years since I worked on a team that had any notion of a structured code review. I previously worked for Microsoft and the team I was part of had a sophisticated process in place to ensure the stability of the code base. This process included a 10 step checklist that a developer had to walk through before they were allowed to check changes into the repository. It didn't matter if you changed only a single line of code, only added a few unit tests or completely rewrote the whole application, you still had to follow the same process before you changes could be committed. One of the items on the checklist was for getting your code peer reviewed. And it usually went something like this
1. The developer sends out an email to 3 other developers and quality assurance members referencing the number of the bug/feature they were working on (no changes were allowed to be made to the codebase without a bug number), and a command (something like "windiff $\vssroot\src \\developers_machine\src") that allowed the recipients of the email to bring up the changes in Windiff on their local PC
2. The developer waited for the team to review the changes, then upon receiving feedback (from at least 2 of them) the developer addressed the items in the email, either saying they incorporated the feedback into the code, or explained why they didn't feel the change was necessary
3. The developer checked in the changes and put the names of the reviewers on the bug for reference
Looking back, there are a number of aspects about this process that I really liked. First, the review is done before the code is actually checked-in - theoretically stopping bad code from infecting the rest of the codebase. Second, at least 2 other people have to review at the code before it makes it in, and finally, 3 people are responsible for the changes since there names are all linked to the bug. Now, at Microsoft we followed this process because we had a large team and any amount of instability lead to unproductive employees, but this level of sophistication would be over-kill for most other development shops. That being said, I don't think it is ok for any size development team (my team is guilty of this too) to not do any type of code review. The last three projects I have worked on did not have any type of formal (or informal) code review process in place, and I believe part of the reason is that there aren't any decent tools that support it (from what I understand Team Systems supports this, but we aren't on it yet).
To help solve this problem, I have been looking for a tool that would meet the following requirements:
1. Integrates with Visual Source Safe
2. Supports command-line or hyperlink or something simple that can be easily sent via email (like the windiff command above)
3. Allows for reviewing diffs before the changes are submitted
Not surprisingly I didn't find anything. As a result, I started playing around with the ss.exe command-line interface (http://msdn2.microsoft.com/en-us/library/39czz3he(...) that ships with VSS to see if it would support some of the primitive operations that I needed to build this tool. It turns out id does, so I created a command line tool (crutil.exe) that accepts 3 arguments on the args[0] = the VSS project path, args[1] = the network share path that maps to the developers PC whose code is being reviewed and args[2] = the VSS user ID of the developer whose changes are being reviewed.
When supplied with these values the tool does the following:
1. Queries VSS (using ss.exe) and requests all of the files changed by the developer (in this example user matt.berseth)
2. Creates a temporary folder on the reviewer's local PC
3. Copies the modified files from the developers PC (\\mattspc\project1\src) to the reviewer's local PC - these files are copied into the local temporary directory
4. Gets the latest version of the changed files from VSS - these files are copied into the local temporary directory
5. Opens WinMerge (http://winmerge.org/) and diffs the directories
M.B.
No comments:
Post a Comment