Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Code Review Workflow + Feature Branching in TFS

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:

  1. The developer works on the feature branch checking-in/shelving and forward-integrating regularly.

  2. When she wants to integrate the feature, she merges back into the baseline and requests a review on these pending changes.

  3. 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 :(

like image 390
pmb Avatar asked Jul 11 '13 12:07

pmb


1 Answers

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:

  1. While all changed files will be linked to the review, the changes since last review won't be highlighted automatically. The reviewer would have to manually do a "Compare to version" and pick the compare target.
  2. There is a limit of 4000 (from the top of my head) files that can be associated to a review, so that may pose a limit to which files you can review as a group (I hope you're not changing 4000+ files between integrations into main).
like image 128
jessehouwing Avatar answered Oct 01 '22 23:10

jessehouwing