locked
TFS for VisualStudio2013 - how to prevent check in without review? RRS feed

  • Question

  • Hello,

    We are looking into how to prevent non-reviewed code from being checked in back to TFS.

    One of the options we explore is to set up a reviewers group and allow that group to check the changes in, but this brings in certain issues like:

     - how to allow checking in someone else's changes

    - should we use shelves for this 

    I was exploring the review policy, but it is not strong enough as it can be overridden

    If you have any experience or know about good articles on this subject could you please share.

    Thank you in advance,

    Andrei



    SSIS question

    Monday, April 18, 2016 3:12 PM

Answers

  • You can't prevent people from checking in low quality code, unless you create a gate, but by creating the gate you limit continuous integration, which is even worse for your code quality and ability to respond to changes and to fix problems. You'll be a constant state of merge hell.

    Instead it's better to instill a culture of quality, organize peer reviews and pair programming as a practice and stress its importance. Allow time for the team to get used to the practice and keep doing it even under time pressure (especially under time pressure).

    Perform regular reviews asynchronously and personally reach out when you detect anomalies. Stress the importance of quality and mentor the people in delivering to the standards you expect of them. In time you'll reap the benefits from this at a much higher level than by taking away people's permissions and taking on the sole responsibility of ensuring quality.

    Quality is a team effort, not a stage gate.


    My blog: blog.jessehouwing.nl

    • Marked as answer by Andreyni Tuesday, April 19, 2016 2:02 PM
    Tuesday, April 19, 2016 11:45 AM

All replies

  • I'd personally recommend against this practice. It is terrible for your teams ability to collaborate and will create bottlenecks. Especially since it's impossible to prevent priviledged users from checking in their own code.

    Technically it's not hard. You'd indeed shelve the code, ask for a review then when the review is done ask another user to check in the shelveset. You can do that from the commandline using

    tf checkin /shelveset:shelveset;owner /author:originaldeveloper


    My blog: blog.jessehouwing.nl

    Monday, April 18, 2016 7:12 PM
  • hi Andrei

    a checkin policy can always be overriden if the user does have the permission to do so.

    you can use a server side plugin which is called when a  checkin is done.

    Turning off policy overrides in TFS

    but don't forget the plugins are version specific.


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

    Monday, April 18, 2016 7:12 PM
  • Hi Jesse,

    Appreciate your response.

    Which practice would you recommend?

    Thanks


    SSIS question

    Tuesday, April 19, 2016 8:41 AM
  • Hi Daniel,

    Thank you for the link. To be honest I'm a bit wary of 3rd party plugins.

    Thanks


    SSIS question

    Tuesday, April 19, 2016 8:48 AM
  • I'd recommend practicing pair programming and if that's not an option at the desk peer reviews where people talk eachother through the code. It beats the quality of any system-driven review and can seriously speed up development.

    Ensure that all mundane checks are automated, so stick Code Analysis, StyleCop, Resharper CLI and other tools into your Continuous integration pipeline. And preferably integrate them into your IDE directly. That way you can focus your review on the structure and the flow of the code and can skip a lot of the simple issues that are otherwise raised.

    If needed you can use a Gated Checkin, though I'm not a fan of those either unless you're able to bring your build time down to a few minutes.


    My blog: blog.jessehouwing.nl

    Tuesday, April 19, 2016 9:07 AM
  • Hi Jesse,

    Thank you for advising.

    We need to review the code created by an offshore team.

    Technically the built in TFS review feature, can somehow be considered as an flavor of peer review.

    However the concern is still there - even if you practice peer review, how do you make sure that none of the developer will cut the corners and check in non-reviewed code? 

    CI with automated mundane checks is a great helper, thank you for mentioning the tooling for this.

    Thanks




    SSIS question

    Tuesday, April 19, 2016 10:45 AM
  • You can't prevent people from checking in low quality code, unless you create a gate, but by creating the gate you limit continuous integration, which is even worse for your code quality and ability to respond to changes and to fix problems. You'll be a constant state of merge hell.

    Instead it's better to instill a culture of quality, organize peer reviews and pair programming as a practice and stress its importance. Allow time for the team to get used to the practice and keep doing it even under time pressure (especially under time pressure).

    Perform regular reviews asynchronously and personally reach out when you detect anomalies. Stress the importance of quality and mentor the people in delivering to the standards you expect of them. In time you'll reap the benefits from this at a much higher level than by taking away people's permissions and taking on the sole responsibility of ensuring quality.

    Quality is a team effort, not a stage gate.


    My blog: blog.jessehouwing.nl

    • Marked as answer by Andreyni Tuesday, April 19, 2016 2:02 PM
    Tuesday, April 19, 2016 11:45 AM
  • Hi Jesse,

    Many thanks for your answer.

    The approach makes sense to me.


    SSIS question

    Tuesday, April 19, 2016 2:04 PM