Sunday, October 20, 2013

Visual Studio Code Review

I have finally been able to work with Visual Studio and TFS code reviews. There were a couple thing that threw me off I wanted to write about. If you want a good quick overview of what this is check out the Channel9 video “Using Code Review to Improve Quality.” There are a few things to note about this feature no one calls out easily:

1- You must have TFS to enable this

2- You must have at least Visual Studio 2012 premium

If you have those two things you can start doing code reviews this way. It is a great feature I think. Sadly it will be under utilized by a lot of development shops for a few reason. First, I think few will understand this feature is there and even if they do fewer will spend the time to defined their processes for it. Second I think a lot of development shops have only a professional license, don’t use TFS as their back, or if they do, they don’t work to use its full capability and just use it as source control.

With all that said if you are using it or thinking of using here are some things I have found out.

Inline code comments

Inline code comment

You can provide comments for the entire change set, document or highlight a line of code and provide comments for just that. However, you can only do inline code comments on code. This means no CSS files, no CSHTML files, etc. Not a huge deal but a bit of a disappointment.

Closing and Sending Comments

Send Comments or Close Reivew options

Once you have provided comments you can send those comments. It is important to note though that is is not the same as closing out your code review (approve or reject). In the above picture you will notice that the “Send Comments” button is grayed out because I have sent my comments. However, the “close review” item is still active. This should allow you to provide feedback on the code without closing out the review. This allows you to have some back and forth with the developer to fine tune the code before you approve the code.

To Check-In or leave outstanding

With code reviews you can request a code review and then either leave your code checked out until code reviews are done or you can check your code in. I am sure everyone has different thoughts here on what they like. I prefer to have developers check-in their code. I prefer this for a few reason. The main reason is code reviews normally don’t happen in a timely manner. I don’t want additional coding or testing slowed down waiting for this. Plus as long as the developer goes back and checks the status of his code review any requested changes can be made then. This is a good reason to have TFS send emails when code review task item status change. It helps make sure developers know comments have been added or a review is completed either because it was declined or it was approved.

You can request a review in a couple ways.

1- Go to “My Work” and under in progress you can select “Request Review”. This is not bad but it means the develop has to remember to go there and request review before checking in.

My work screen for request review

2- Request review while checking in. This can be done by selecting the “Actions” link.

pending changes screen for request review

Code reviews are only good if they happen. I am sure there are lots of opinions out there on how to make code reviews happen. I don’t think there is one answer for this problem. TFS does not support (out of the box) a check-in policy that says you have to have had or requested a code review. Personally I am not a big fan of that approach anyway but I don’t think every check-in needs a code review. This is where the SDLC process creation and management comes into play. If your team is not active in creating, managing, enforcing and refining their SDLC rules this will not be as beneficial as it could be to you.

1 comment:

David Kreth Allen said...

Thanks for highlighting the points of concern about CSS, HTML. And I agree about the need for SDLC focus. Most shops don't pay attention to their processes.