How can we improve Team Services?

TFS code review should not require a new code review to be started when code that is already under review is changed based on review comment

It seems that TFS requires a new code review to be started whenever code that is already under review is updated in response to review comments. It is not only unnecessary busy work for a developer to initiate a new review each time a review comment would suggest a change to the code that is already under review is necessary, but it also prevents having a single picture of changes made as a result of the review process.

With the current method, a single logical code review may span multiple physical code reviews and the comments for the single logical review are fragmented across several physical reviews.

2,116 votes
Vote
Sign in
Check!
(thinking…)
Reset
or sign in with
  • facebook
  • google
    Password icon
    I agree to the terms of service
    Signed in as (Sign out)
    You have left! (?) (thinking…)
    MichaelMichael shared this idea  ·   ·  Flag idea as inappropriate…  ·  Admin →
    SamiSami shared a merged idea: Make code review feature usable  ·   · 

    54 comments

    Sign in
    Check!
    (thinking…)
    Reset
    or sign in with
    • facebook
    • google
      Password icon
      I agree to the terms of service
      Signed in as (Sign out)
      Submitting...
      • Rob SiklosRob Siklos commented  ·   ·  Flag as inappropriate

        @Andrew Ch: This is not a a reliable of doing things. The problem is that as the comments get addressed, the line numbers change, and you complete lose sight of what the original code review captured.

        @80's Rocker: Couldn't agree more, but the writing is clearly on the wall - TFSVC is dead, and too bad for all of us that are using it. I guess we're just expected to migrate to Git or stay with TFSVC and live with a reduced feature set. I wish they'd just come right out and say this. At the end of the day, it seems the mob has spoken and Git is the winner.

      • 80's Rocker80's Rocker commented  ·   ·  Flag as inappropriate

        This is really sad that it on 1/15/16 this was still not on the roadmap to be implemented. Greate feature is being killed by poor implementation and not fixing a glaring feature shortcoming in a timeley mannner. The VS team obviosly uses GIT and that is why getting this implemented in GIT was a big priority instead of TFS. But by doing that they are ignoring a very big user base of TFS who don'want to switch over to GIT.

      • Andrew ChAndrew Ch commented  ·   ·  Flag as inappropriate

        There seems to be a way of doing this already -- if you make your changes in response to the code review, and then save the changes back to the shelveset (set the name to the same name of the shelve set used for the code review).

      • ScottScott commented  ·   ·  Flag as inappropriate

        I agree with Joshua, moving to git is just not an option for us at this point. We have many in house build process(es) built on top of tfs vc. This really needs to be planned and implemented and not just abandoned for git.

      • Brendan H.Brendan H. commented  ·   ·  Flag as inappropriate

        Just a comment that this is working well for us now using TFS Git on premise! We do not use TFVC.

      • Vito DeCarloVito DeCarlo commented  ·   ·  Flag as inappropriate

        I agree. I'm not against git, but if that's where everything is heading, I'd just assume migrate now and get it over with.

      • StephenStephen commented  ·   ·  Flag as inappropriate

        Am I reading this right? That this is being done for Git, but not TFVC?

        If so, I am a bit concerned. I did not believe the "TFVC is dying" statements. But if this is being done for Git and is not even road-mapped for TFVC, well that looks bad.

        Makes it looks as if TFVC is a "second class" member of the team. At best.

      • Keith CulpepperKeith Culpepper commented  ·   ·  Flag as inappropriate

        Any update on this? I agree that the code review should be updated instead of having to create a new review.

      • David CoulterDavid Coulter commented  ·   ·  Flag as inappropriate

        Put hover text on ALL grayed out items, so we will know how to ungray them and use them!!! you can start with "Request Review".

      • Mathias BroikmanMathias Broikman commented  ·   ·  Flag as inappropriate

        2) Is needed to make code review usable.

        "I really sometimes wonder wether VS team eats its own dog food. I doubt that the feature is usable in any non-trivial project."
        I agree

      • Matt LuceenMatt Luceen commented  ·   ·  Flag as inappropriate

        My team is using the "update shelveset" method posted below and we hit a snag. Here's the scenario and two workarounds for the snag we hit:

        SCENARIO
        1. Person 1 requests a code review from a shelveset.
        2. Person 2 finds issues, adds comments, and selects "Finished (Needs Work)".
        3. Person 1 makes the necessary changes and pushes them to the same shelveset used in step 1.
        4. Person 1 adds a comment to the code review to notify the team that the code is ready to be reviewed again.
        5. SNAG: Person 2 reviews the new code and it looks good. However, Person 2 has already finished the code review -- they are unable to select "Finished (Looks Good)".

        WORKAROUND 1 - Don't ever finish a code review if the code needs work
        Don't ever use "Finished (Needs Work)" or "Finished (With Comments)". If the code needs work, Person 2 should leave a "-1" top level comment on the code review and leave file level comment(s) describing the work needed.

        WORKAROUND 2 - Use Excel to reset Person 2's response
        1. Open the code review in Visual Studio.
        2. Under Actions, click Open Work Item.
        3. Click the LINKS tab on the bottom left of the work item.
        4. Right-click Person 2's "Finished (Needs Work)" response and open it in Excel.
        5. Click into the Assigned To cell and type Person 2's name.
        6. Click into the State cell and use the dropdown to select "Requested".
        7. Close Excel, clicking Yes to confirm that you want to publish your changes to the team project.
        8. Person 2 is now able to select "Finished (Looks Good)".

      • Elvis Miranda AramayoElvis Miranda Aramayo commented  ·   ·  Flag as inappropriate

        Exactly my thoughts, we need this feature implemented ASAP, as a code reviewer find myself quite limited without this functionality.

      • Stefan SieberStefan Sieber commented  ·   ·  Flag as inappropriate

        We have given up waiting for code review to become usable. 2) was a complete killer for us. We're now using review assistant and it meets our requirements.
        I really sometimes wonder wether VS team eats its own dog food. I doubt that the feature is usable in any non-trivial project.

      • Ryan TurnerRyan Turner commented  ·   ·  Flag as inappropriate

        Could not agree more with the original suggestions. These are all major pain points for developers. Managing multiple reviews of the same set of 10+ files becomes unfeasible with the current tool. Not being able to maintain a history within a single review and see diffs between different versions of files makes the code review process time-consuming, and traceability becomes a nightmare.

      ← Previous 1 3

      Feedback and Knowledge Base