Does everyone need to have code reviews performed?

Apr 13, 2010 at 9:19 PM
Edited Apr 13, 2010 at 11:00 PM

First off let me say thanks for all your efforts on the project JB.

The company I work for has recently decided to revisit our on-boarding process for new developers. I came across your project and it seems solid, but I just had a couple concerns that needed to be addressed before I could do anything else with it. We have a rather large staff of developers who we trust committing changes directly to the source repository. However, the new developers we bring on need code reviews performed until each demonstrates a solid understanding of the code and are able to produce high quality code on their own. This brings me to my questions, does TeamReview allow specific users or groups to be excluded from code reviews? Also, is it possible for a developer to bypass their need for code reviews to be performed on their check-ins?

Thanks again for all your hard work, I look forward to hearing your response!

- Jeff

Apr 14, 2010 at 1:03 AM

Hi Jeff,

I'll take the opportunity to answer your questions.

TeamReview does not enforce a check-in policy. It is more of a workflow type scenario - performing reviews of code and replaying a code review.

If you would like a specific check-in policy to apply to certain user/group , then a custom TFS check-in policy would need to be created.

Last time I looked, TFS check-in policy can be overridden, by ticking the override policy and supplying a comment.

It is possible to receive email notifications of policy override checkins from TFS.

See this msdn article as a starter to custom check-in policies:- http://msdn.microsoft.com/en-us/library/ms181281.aspx

To create a custom checkin policy in code, create a project, reference Microsoft.TeamFoundation.VersionControl.Client.dll

Create a class which inherits from PolicyBase and override the methods for your purposes.

Hope this helps

 

Apr 14, 2010 at 5:21 AM

Yeah I've written policies before, I actually have one hosted on CodePlex for StyleCop. So just so we're straight here, there is no existing way in TeamReview to say which users of a project are supposed to do a code review?

Apr 14, 2010 at 6:03 AM

I'm assuming you mean people within a project, who upon source code checkin, are required to have either requested a code review or completed a code review first?

No, Teamreview does not stop checkins. (In my mind it would be more of a custom checkin policy specific to a project/site.)

Do you utilise shelvesets in the interim? (ie. coder shelves changes until review completed, then has approval to checkin).

 

If my assumptions above are incorrect, explain what you mean by >>"supposed to do a code review?" << , ie. :-

1) A reviewer who creates the "New Code Item" or

2) a coder who replays the Code Review Item and makes changes to source code as a result of a code review?

A reviewer is potentially anyone in a project team. Team review doesn't impose any limitation on who can create a review.

cheers

Apr 17, 2010 at 3:22 PM

Right now we have no process in place for code reviews, we've just been exploring options how we can handle this.

When I said "supposed to do a code review" I was referring to the developer who modified or created the code and needs to have someone review his/her work. Also, we have large amount of users on the TFS server and you mentioned it's not limited who reviews the other persons work, it might be useful if we can have a way of limiting that. It's difficult when creating work items who to assign them to because the user base in the TFS server is so large.

I can always write the TFS policy myself, if I do would you like to include it as part of the project? It wouldn't need to be enforced, but having the option available is always nice. The biggest difficulty with TFS Check-in Policies is they can be bypassed (which I'm somewhat of the enforcer on our team ensuring people don't do that), and crossing versions of Visual Studio and TFS cause other issues. For example, using VS 2005 on a TFS 2008 server will cause the VS client to load the TFS 2008 Explorer client assemblies which is not possible because VS 2005 cannot load those assemblies.