We are starting to work with feature branches and we want to set up a check-in policy that only allows check-ins to the baseline when they have an associated code review.
The new code review workflow in 2012 is quite nice, since you can easily interact with the developer and other reviewers, and comment lines of code directly. Nevertheless it seems like MS didn't think the use case fully because we easily run into the following problem:
The developer works on the feature branch checking-in/shelving and forward-integrating regularly.
When she wants to integrate the feature, she merges back into the baseline and requests a review on these pending changes.
The reviewer makes several comments and now she has to change some code. Where does she do this?
Option 1: Go back to the branch, edit the code and check-in the changes in the branch. Undo the pending changes of the first merge. Merge and request a review again. Repeat until there are no more comments. Check-in the merge. This is not so nice because all the review comments are in the pending changes of the merge, and she has to work on the branch where she doesn't see the comments directly.
Option 2: Make the edits directly on the pending changes of the merge. Request a review again. Repeat until there are no more comments. Check-in the merge. If she wants to continue working on the branch, she would have to make a forward integration because the changes from the review are not there.
Either way, the second review is always very annoying, because you have no way of only seeing the changes between the first and the second review, since you are always diff-ing with the baseline.
Am I missing something here? Is there another option that allows reviewing the changes from a review? Does anyone have a better way of feature branching and code reviewing?
New: Using VS and TFS2013, still no improvements :(
You're not missing anything. This is an unfortunate problem associated with the way Code Reviews were implemented, they can only be linked to one changeset, not to a range of changes.
If your team is used to a high frequency of checkins on their feature branches, then having every individual changeset reviewed using the tool may be a lot of overhead. But that would be my recommendation.
There is a trick, it's not ideal, but it may help. You can check out (on your feature branch) all files that were changed since the last checkin. Then request a review. It will create a shelveset with your changes and associate that to the review. That way you don't have to perform the merge prior to requesting the review. Just make sure you merge the latest version from main into your feature branch before pulling this trick. There are 2 major drawbacks to this: