locked
Code Review in TFS 2012 only after task completed RRS feed

  • Question

  • Hi.

    Is there a way to enforce a code review on tasks in the sprint as they move from active to complete/resolved?

    My co-workers and I check code in all day and we want to do a code review but not after every single check in. It would be stupid to enforce a code review after checking in 4 lines of code. Can we create a custom code review after the task from the Sprint is complete. Or after the user story is complete?


    chuckdawit

    Thursday, January 31, 2013 11:53 PM

Answers

  • The Code Review experience is on a per-changeset basis. Thus per checkin. There are several ways you can work with TFS to integrate the Code Review workflow, they're not ideal if you check in on one branch at high frequencies.

    1. Work on a feature branch to develop your code, when merging, request a checkin on the merge, instead of on all changesets individually. This allows you to work with multiple people on the same same functionality and still integrate the Code Review workflow
    2. Work on a shelveset (use suspend/resume functionality at the end of the day). Request a Code Review when you've finished a feature completely. This allows one person to work on a piece of functionality for an extended period of time. The Shelvesets will ensure that the code is not lost when a machine crashes. You can request a Code Review just before checkin in the finished work.
    3. Request your review post checkin. This is my preferred method anyway, as it doesn't slow down the team. You can go to the History panel, rightclick a changeset and request a review on it.

    There's actually a nasty trick you can use to request a review on a set of  files, without actually checking in anything.

    • In a clean workspace, with all files at their latest version, check out each individual file you want reviewed individually (this requires a server workspace). With all files you want to include in the review checked out, request review. A review request will be created with all checked out files (regardless of any changes made to them)
    • Undo all pending changes in your local workspace.

    My blog: blog.jessehouwing.nl

    • Marked as answer by witdaj Thursday, February 7, 2013 4:42 PM
    Thursday, February 7, 2013 11:23 AM

All replies

  • Hi Chunkdawit,

    Thank you for your post.

    According to your description of the issue, here is one situation I want to clarify form you: What TFS you are using now, on-premises TFS 2012, or team foundation service?

    As far as I know, Code review is for pending changes, if check in all pending changes, I cannot request a code review. In other words, there are some pending changes I want someone review them, I can request a code review. Of cause, if all changes in the sprint are not checked in, you request a code review at last, all pending changes may contained in the code review, but this is not sensible. I suggest you submit a feedback to Visual Studio UserVoice site (http://visualstudio.uservoice.com/forums/121579-visual-studio).

    Regards,


    Lily Wu
    MSDN Community Support | Feedback to us
    Develop and promote your apps in Windows Store
    Please remember to mark the replies as answers if they help and unmark them if they provide no help.

    Friday, February 1, 2013 7:24 AM
    Moderator
  • hi lily,

    I'm still new to TFS and have another question. As far as I understand a changeset is what gets checked-in?

    And in a change-set you can have multiple files changed and you can have checked in multiple files before you check in the changeset?

    I don't think I fully understand this area. is only the changeset checked in while everything else just gets checked in only the local machine?


    chuckdawit

    Monday, February 4, 2013 7:22 PM
  • hi lily,

    I'm still new to TFS and have another question. As far as I understand a changeset is what gets checked-in?

    And in a change-set you can have multiple files changed and you can have checked in multiple files before you check in the changeset?

    I don't think I fully understand this area. is only the changeset checked in while everything else just gets checked in only the local machine?


    chuckdawit

    please take a look the Visual Studio ALM Glossary: http://msdn.microsoft.com/en-us/library/ms242881%28v=vs.100%29.aspx

    changeset

    A logical grouping of changes. The purpose of changesets is to group all of the file and work item updates that get delivered with a single check-in action.


    Please use Mark as Answer if my post solved your problem and use Vote As Helpful if a post was useful.

    Monday, February 4, 2013 7:31 PM
  • Hi,

    I'm using a server located on premises. I'm not using the online service.

    So from what I see in Daniels answer I can create a changeset without checking-in? And then when I want to I can submit and check in everything from that changeset and perform the code review on everything in the chageset?


    chuckdawit

    Wednesday, February 6, 2013 5:35 PM
  • So from what I see in Daniels answer I can create a changeset without checking-in?

    no - a changeset groups changes from multiple files together when checked-in with a single check-in action.

    And then when I want to I can submit and check in everything from that changeset and perform the code review on everything in the chageset?

    no. CodeReview works only on pending changes and not on a changeset. Pending changes are locally modified files, files in a shelveset. When you request a CodeReview in VS2012 Premium/Ultimate you can select what type of of pending changes you want to review - if you're selecting local files and not a shelveset VS2012 will create a shelveset internally - I assume.

    CodeReview field in the check-in dialog is only a simple comment and is not comparable to CodeReview in VS2012 Premium/Ultimate in the Team Explorer windows.


    Please use Mark as Answer if my post solved your problem and use Vote As Helpful if a post was useful.

    Wednesday, February 6, 2013 7:28 PM
  • hi Daniel,

    I'm trying to determine if my group wants to use the code review of TFS. I'm trying to get them to use it but seeing that you perform a code review everytime you check something in seems like a waste of time, especially if we are checking in throughout the day and only checking in small bits of code at a time. (Say 4 lines) I wouldn't want to do a code review on 4 lines of code 7 times a day. Is there anotherway of looking at this and performing a code review on a larger section of code? You mention the shelveset? Would the methodology in a code review on a shelveset be - store files in a shelveset and don't merge, then once ready (ex. user story complete/work item task complete) check in shelveset and THEN perform a code review? is this possible to perform a code review on a checked in shelveset?

    I'm not referring to the comment here. We have VS 2012 Ultimate and TFS 2012


    chuckdawit

    Wednesday, February 6, 2013 10:59 PM
  • Hi Chunkdawit,

    Code review is not required for each check in. If you want someone review the code changes, you can request a code review before check in, you can check in the changes after review passed.


    Lily Wu
    MSDN Community Support | Feedback to us
    Develop and promote your apps in Windows Store
    Please remember to mark the replies as answers if they help and unmark them if they provide no help.


    Thursday, February 7, 2013 7:10 AM
    Moderator
  • The Code Review experience is on a per-changeset basis. Thus per checkin. There are several ways you can work with TFS to integrate the Code Review workflow, they're not ideal if you check in on one branch at high frequencies.

    1. Work on a feature branch to develop your code, when merging, request a checkin on the merge, instead of on all changesets individually. This allows you to work with multiple people on the same same functionality and still integrate the Code Review workflow
    2. Work on a shelveset (use suspend/resume functionality at the end of the day). Request a Code Review when you've finished a feature completely. This allows one person to work on a piece of functionality for an extended period of time. The Shelvesets will ensure that the code is not lost when a machine crashes. You can request a Code Review just before checkin in the finished work.
    3. Request your review post checkin. This is my preferred method anyway, as it doesn't slow down the team. You can go to the History panel, rightclick a changeset and request a review on it.

    There's actually a nasty trick you can use to request a review on a set of  files, without actually checking in anything.

    • In a clean workspace, with all files at their latest version, check out each individual file you want reviewed individually (this requires a server workspace). With all files you want to include in the review checked out, request review. A review request will be created with all checked out files (regardless of any changes made to them)
    • Undo all pending changes in your local workspace.

    My blog: blog.jessehouwing.nl

    • Marked as answer by witdaj Thursday, February 7, 2013 4:42 PM
    Thursday, February 7, 2013 11:23 AM
    1. Work on a feature branch to develop your code, when merging, request a checkin on the merge, instead of on all changesets individually. This allows you to work with multiple people on the same same functionality and still integrate the Code Review workflow

    This is exactly what we are trying to do, but we came across the following situation:

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

    2. When she is done, she merges back into the baseline and requests a review on these pending changes.

    3. The reviewer make several comments and 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 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.

    I personally like option 2 better. Either way, the second review is always very annoying, because you have no way of seeing only the changes between the first review and the second, since you are always diff-ing with the baseline.
    I think that the review of changes from a review and feature branching is something that should be better integrated into future TFS code review workflows.


    Thursday, July 11, 2013 12:17 PM