Skip to main content

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.

Comments

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.

Popular posts from this blog

MVVM light and Model Validation

I have been using the MVVM light toolkit for a project recently. It is a great toolkit but is missing a couple things and Laurent Bugnion does a good job trying to cover those holes. One of the things the toolkit does not support is Validation. The good news is there is a great CodePlex project out there call Fluent Validation that makes this pretty easy to add and really powerful. My objective was to add validation to my model so I could call “IsValid” on the model itself (similar to the MVC attribute approach). Fluent Validation has you create a new class file that holds you validation rules for a given model. This is the approach I took to enable each model to have an “IsValid” property and a “Errors” property that returns the validation errors.First I setup my ValidationFactory:publicclass ValidatorFactory : FluentValidation.ValidatorFactoryBase{publicoverride FluentValidation.IValidator CreateInstance(Type validatorType) {return SimpleIoc.Default.GetInstance(validatorType) as …

Experience Profile Anonymous, Unknown and Known contacts

When you first get started with Sitecore's experience profile the reporting for contacts can cause a little confusion. There are 3 terms that are thrown around, 1) Anonymous 2) Unknown 3) Known. When you read the docs they can bleed into each other a little.

First, have a read through the Sitecore tracking documentation to get a feel for what Sitecore is trying to do.

There are a couple key things here to first understand:

Unless you call "IdentifyAs()" for request the contact is always anonymous. Tracking of anonymous contacts is off by default. Even if you call "IdentifyAs()" if you don't set facet values for the contact (like first name and email) the contact will still show up in your experience profile as "unknown" (because it has no facet data to display).  Enabled Anonymous contacts


Notice in the picture I have two contacts marked in a red box. Those are my "known" contacts that I called "IdentifyAs" on. I know they say &…

Uniting Testing Expression Predicate with Moq

I recently was setting up a repository in a project with an interface on all repositories that took a predicate. As part of this I needed to mock out this call so I could unit test my code. The vast majority of samples out there for mocking an expression predicate just is It.IsAny<> which is not very helpful as it does not test anything other then verify it got a predicate. What if you actually want to test that you got a certain predicate though? It is actually pretty easy to do but not very straight forward.Here is what you do for the It.IsAny<> approach in case someone is looking for that. this.bindingRepository.Setup(c => c.Get(It.IsAny<Expression<Func<UserBinding, bool>>>())) .Returns(new List<UserBinding>() { defaultBinding }.AsQueryable()); This example just says to always return a collection of UserBindings that contain “defaultBinding” (which is an object I setup previously). Here is what it looks like when you want to pass in an expressi…

Excel XIRR and C#

I have spend that last couple days trying to figure out how to run and Excel XIRR function in a C# application. This process has been more painful that I thought it would have been when started. To save others (or myself the pain in the future if I have to do it again) I thought I would right a post about this (as post about XIRR in C# have been hard to come by). Lets start with the easy part first. In order to make this call you need to use the Microsoft.Office.Interop.Excel dll. When you use this dll take note of what version of the dll you are using. If you are using a version less then 12 (at the time of this writing 12 was the highest version) you will not have an XIRR function call. This does not mean you cannot still do XIRR though. As of version 12 (a.k.a Office 2007) the XIRR function is a built in function to Excel. Prior version need an add-in to use this function. Even if you have version 12 of the interop though it does not mean you will be able to use the function. The a…

Security Config in IIS Express

I have gotten tired of always having to look this up or remember where it is at. That means it is time to post to my blog so I can find it easier and hopefully others can too. If you are having issues with IIS Express authentication errors (like the Unauthorized 401.2 error I always get) here is some help. I can never remember what the last setting was I had IIS Express set to for authorization. To change IIS Express for windows auth or anonymous auth you want to work with the applicationhost.config file. It can be found here …Documents\IISExpress\config. You want to change the settings in the following area of the config file. <authentication><anonymousAuthenticationenabled="true"userName=""/><basicAuthenticationenabled="false"/><clientCertificateMappingAuthenticationenabled="false"/><digestAuthenticationenabled="false"/><iisClientCertificateMappingAuthenticationenabled="false"></iisCli…