It's all about the answers!

Ask a question

Code review - best practices / how do you do it?


Anton Piatek (3196) | asked Jan 14 '11, 3:57 a.m.
Hi,
I am looking into how exactly we should be doing our code reviews with RTC 2. I have been reading the docs which show how to turn on the requirements for a work item and a review, but little more about how exactly to configure the review process.

In our old code control tool we simply let each person choose their buddy-checker, and then that person was responsible for either delivering the code, or sending it back for rework.

I heard of an idea that is apparently possible in RTC, but I am not sure how to set the permissions:
The developer chooses their reviewer and puts the work item up for review. The review can optionally add an extra changeset to the work item, which contains any extra small changes. The developer can then either deliver both changesets together, or discard the reviewer's work and redo their own changeset and submit for review again.

This idea sounds pretty good for those small changes that often come out of a code review, but I am not sure how to set the config to achieve this.

What other ways have you found that make for better code reviews? What areas are worth locking down (i.e. should the developer be able to add more changesets after review but before delivering the work item to the stream) ?

What areas did you find are worth leaving more flexible (locking things down doesn't always make life easier)

3 answers



permanent link
Tim Mok (6.6k38) | answered Jan 14 '11, 8:41 a.m.
JAZZ DEVELOPER
Hi,
I am looking into how exactly we should be doing our code reviews with RTC 2. I have been reading the docs which show how to turn on the requirements for a work item and a review, but little more about how exactly to configure the review process.

In our old code control tool we simply let each person choose their buddy-checker, and then that person was responsible for either delivering the code, or sending it back for rework.

I heard of an idea that is apparently possible in RTC, but I am not sure how to set the permissions:
The developer chooses their reviewer and puts the work item up for review. The review can optionally add an extra changeset to the work item, which contains any extra small changes. The developer can then either deliver both changesets together, or discard the reviewer's work and redo their own changeset and submit for review again.

This idea sounds pretty good for those small changes that often come out of a code review, but I am not sure how to set the config to achieve this.

What other ways have you found that make for better code reviews? What areas are worth locking down (i.e. should the developer be able to add more changesets after review but before delivering the work item to the stream) ?

What areas did you find are worth leaving more flexible (locking things down doesn't always make life easier)

You can set up RTC to be similar to your process with your old code control tool. The permissions you are looking for are set in a project/team area's process configuration under Team Configuration > Operational Behavior > Source Control > Deliver.

You can add Require Work Item Approval to your process. This will check on delivery if the associated work item has the required approvals. Developers can deliver when approvals are complete or approvers can discard the changes from stream if the approval is rejected.

Your team may want other process checks added. I think it's ok to add what your team thinks is necessary for proper control of what goes into the stream. The checks can even be configured per iteration. This allows for less restrictive checks early on in the timeline and for more restrictive checks later on when you start locking down your release.

permanent link
Anton Piatek (3196) | answered Jan 14 '11, 10:14 a.m.
I am not sure I want to just "do the same we had before" - It wasn't necessarily designed, as the easiest that the old system let us do.

How can I configure who is required to have done a code review? Some people have asked about a 2 stage review - i.e. the person next to you does a review, and then the department expert does a quick review to check nothing was missed.

permanent link
Tim Mok (6.6k38) | answered Jan 14 '11, 10:55 a.m.
JAZZ DEVELOPER
I am not sure I want to just "do the same we had before" - It wasn't necessarily designed, as the easiest that the old system let us do.

How can I configure who is required to have done a code review? Some people have asked about a 2 stage review - i.e. the person next to you does a review, and then the department expert does a quick review to check nothing was missed.
When you configure the Require Work Item Approval condition, you must specify the type of approval and # of role type that must be present in order to deliver. The developer still has to pick the person that has to perform the review. The restriction is the selected approver must match the condition (ie. developer cannot deliver if the selected approver is another developer when the condition is department head as approver).

Your answer


Register or to post your answer.


Dashboards and work items are no longer publicly available, so some links may be invalid. We now provide similar information through other means. Learn more here.