Code Reviews using TFS Tools

I've previously blogged about using code reviews as an effective way to minimize defects, improve code quality and keep code more maintainable.  In my new job I had to define a code review process that suited our development team as well as the Team Foundation Server toolset and the Conchango SCRUM for Team System process template we are using.  This post details what process and tooling we came up with.

Review Style

To improve on the cost-benefit factor of the reviews we opted for a lightweight code review process.  The aim was to get improved code quality but not incur the overhead of traditional meetings-based inspections.  Because of the distributed nature of our development team, we are using a flavour of the e-mail pass-around review style.  In a traditional email pass-around, the author or source code management system packages the code changes and sends an e-mail to the reviewers.  We decided to use the Code Review Sidekick of Attrice Corporation to get a consolidated list of code changes that have been checked into TFS.  We do not send an e-mail containing the changes but instead use a Code Review Sprint Backlog item to track and manage the review process.  More on this in the Review Process section.

Review Frequency

We do reviews after all the development tasks required to complete a piece of business functionality have been checked in. We discussed the issue of using shelving to prevent not reviewed code from being checked in, but the team felt that as we are following a quite rigorous process for check-in we can do the review at a later stage on the checked-in changes.  The developers working on completing their tasks are however not allowed to change the status of their tasks to Done.  The reviewer has the responsibility of signifying the work as completed when he/she has completed a successful review.  To make the review process as light as possible we also try to automate a lot of the coding standards and architectural standards through the use of tools like FxCop, ReSharper and NDepend.

Review Tooling

Although there are some CodePlex projects that provide support for customizing TFS to support code reviews and code review flows, we decided to steer away from using these solutions as they depend on creating custom work items and custom work item states.  We wanted to only use the default Conchango SCRUM for Team System process template.  Unfortunately the template doesn't contain a Ready for Review state for a Sprint Backlog item, but we decided to use the Ready For Test state to signify that a task is ready for review as we execute the testing as part of the check-in.

Review Procedure

As already mentioned, to manage and track the code review process and feedback, we create a special Code Review Sprint Backlog Item (SBI) associated with every Product Backlog item (PBI) during Sprint planning.  The author ensures that all the development SBI's for the PBI are in the Ready for Test status and have been checked in.  The author marks the Code Review SBI as Ready for Test and assigns it to the reviewer to request a review.  The reviewer gets notified of the new review work item assigned to him/her and uses the Code Review Sidekick tool to get a consolidated list of changesets with their associated files for the work completed.

This presents the review with a list of the code changes that were made.

When the reviewer right clicks on a file to review, he/she can have up to 3 options.

The options imply the following:

1. Compare With Previous (Not In View) – This will compare the versions of LogonController.cs in CS 6870 and CS 6721 to each other.  Use this option to see what has changed within the file as part of the complete development effort associated with the PBI work.
2. Compare With Previous In View – This will compare the versions of the LogonController.cs in CS 6870 and CS 6869 to each other (only if > 1 change to same file).  Use this option to see what has changed within the file between the two changesets.
3. Compare With Oldest in View - This will compare the versions of the LogonController.cs in CS 6870 and CS 6722 to each other (only if > 1 change to the same file).  Use this option to see what has changed between the first and last checkin for the same file.

The reviewer conducts the review and continuously updates the Work remaining on the Code Review SBI.  Any questions are clarified by the author through Skype/Interwise or verbal communication. The reviewer also adds any comments/notes to the Code Review SBI itself. After completing the review, the reviewer contacts the author to discuss the issues identified. The author and reviewer agrees upon what issues needs to be addressed.  Here we can have 2 possible outcomes:

1. Small fixes - author quickly fixes these and checks in the changes
2. Major fixes
   1. The reviewer assigns the Code Review SBI back to the author to make the relevant fixes.
   2. The author updates the Work Remaining of the relevant development SBI's to a value > 0 to indicate the effort required to make the fixes.  This shows up on the Sprint Burndown chart as an upward blip as the effort for the sprint increases due to the rework.
   3. The review process is repeated until the reviewer is satisfied

Finally, once the reviewer is satisfied, the reviewer changes the Code Review SBI status to Done and changes the status of the development SBI's to Done to signify that the PBI is completed.