Code reviews for larger ASP.NET MVC team using TFS

Question

I'm trying to find a good code review workflow for my team. Most questions similar to this on SO revolve around using shelved changes for the review, however I'm curious about how this works for people with larger teams.

We usually have 2-3 people working a story (UI person, Domain/Repository person, sometimes DB person). I've recommended the shelf idea but we're all concerned about how to manage that with multiple people working the same feature. How could you share a shelf between multiple programmers at that point? We worry it would be clunky and we might easily have unintended consequences moving to this workflow.

Of course moving to shelfs for each feature avoids having 10 or so checkins per feature (as developers need to share code) making seeing the diffs at code review time painful.

Has anyone else been able to successfully deal with this? Are there any tools out there people have found useful aside from shelfs in TFS (preferably open-source)?

Answer 1

Is the team distributed? If not, then perhaps just try some XP techniques like pair programming (even if not all the time) and get the developers to work together on a feature. It is usually better to communicate continuously than to designate specific events (like code reviews) when problems are found.

Answer 2

The main caveat to using shelvesets with TFS is that you can't do diffs between shelvesets on different branches. You can only diff to the branch the shelveset was created from.

Also, the support for updating your current changes with changes made by others to the shelveset is not great. When you pull a shelveset onto the local machine, the default action is to overwrite the local content, not merge. I'm not sure if merge is even supported there.

So having multiple people making changes to the same shelveset could be awkward and risk overwriting local work when updating from the server.

For TFS, you will probably be better served by using a branch for each mini-team, as you get full merge support in all directions that way.

A few other version control systems (GIT, possibly StarTeam) have the notion of a "personal branch". It's like TFS's shelveset but it behaves more like a true branch in that you can merge to and from it, unlike TFS shelvesets.

Side note: don't be afraid of multiple smaller checkins. You can diff ranges of checkins at one time to coalesce multiple checkins into one diff. As a matter of personal style, I prefer to check in small edits often rather than sit on a monster revision for days or weeks (or months!). Big revisions take longer to review, incur a greater risk that something will be overlooked or break, incur a much higher probability that you will have to manually resolve merge collisions, and you lose the 1:1 tracking atomicity of one bug, one checkin.

Holding your revisions for one big checkin is sometimes referred to as the "constipated" workflow. ;>

Answer 3

Some follow up questions; how many stories are you running concurrently, how big is the total team, can you do smaller iterations (even if you don’t release)?

Not yet knowing the answers I would say;

If the complexity of the stories doesn’t require different branches, how about just having a full code review of all the changes near the end of the iteration? This way you see not only all the changes but how they work together. If people can’t own up to the changes they made during that review you can always find the change set (but at that point you really have bigger team issues). Depending on the size of your team this may not be practical.

One last thought; good test coverage and continuous integration are life-savers.

Answer 4

I was able to get TFS and ReviewBoard working.

http://blogs.pinet.ca/tim/2010/08/24/code-review-goodness-with-reviewboard/

Tim