Jazz Community Site - Jazz Team Blog » Reviewing code changes with Rational Team Concert

Christophe Cornu

rational-team-concert

rtc

scm

source control

Tips and Tricks

video

workitems

Thu, 11 Oct 2012

9 min read

Not rated

In the good old days, things were easy. You worked on a bug, modified a few lines of code and were done. No one was looking over your shoulder to check if your code was right. The best code comes late at night when you’re alone anyway. No tests to run and no continuous build that would catch all the regressions you may have introduced. I dare you to say that in your career you have never seen this situation… And I don’t need to know if you were the late at night breaking code genius developer or the early morning one trying to repair :-)

Luckily things have changed. Rational Team Concert makes it easy to ask one of your peers to review your changes when you close a defect. In the scenario below, a developer named Christophe has completed his work on a Task. He would like his teammate Tim to review his changes. Christophe goes to the Approvals tab in the Work Item editor and adds an approval for Tim.

Tim receives a notification by email or through an RSS feed (e.g. in the Team Dashboard). He opens up the Task and goes through the summary and comments to get some context. When he understands what Christophe was asked to do, he switches to the Links section in the work item editor and opens the associated change sets.

After clicking Open, the Change Summary view lists all the files and folders that Christophe had modified in these change sets. Tim sees that Christophe has modified 2 files.

Tim double clicks on the first file named MobileStartup.java (or right click and ‘Open in Compare Editor’). A compare editor opens up, showing all the changes Christophe made to that file when all the three change sets are applied. The Change Summary view aggregates all the change sets to present a synthetic view of all the resources modified. It gives access to a compare editor that visualizes how all these change sets affect the selected file. Tim is satisfied with the changes to that first file.

Tim now reviews the second file, MobileWebView.java. He likes some of the changes, but some others seem unrelated to the Task he is reviewing. In need of clarity, he decides to turn on the Details Pane.

The Details Pane shows that Christophe modified MobileWebView.java in three change sets. The Dependency Graph tells Tim how these change sets relate to each other. Tim can also see the comment of each change set and he gets suspicious of the second one. He double clicks on that second row. He inspects in a compare editor how that particular change set affected MobileWebView.java (in isolation of the changes from the two other change sets). The code is manifestly incorrect and not related to that task. Tim rejects the Review and adds an explanatory comment for Christophe in the work item editor (with a @Christophe).

Christophe agrees with Tim and attaches a fourth change set correcting the bad code he introduced in that file (Christophe could also create a change set to replace all previous three change sets if things were really bad). He asks Tim to review again. This time, Tim is happy to approve.

In this example, Christophe was a well behaved teammate. He asked Tim to review his changes. But what if Christophe is distracted and forgets to ask for a review? The answer is to customize the process to add appropriate rules that work for your team and industry. Customize the project area or team area that Christophe is part of (i.e. the project area or team area that owns the stream Christophe delivers his change sets to). Here are two popular preconditions to set.

Require Work Items and Comments – to ensure Christophe associates a work item to every change set he delivers
Require Work Item Approval – so that Christophe cannot forget to set a review request on the work items associated to the change sets he delivers

There are many fancy ways to customize the process to your liking. You can enforce that a certain copyright be included, control which role is required for a review, etc. Evan’s article Source Control process recipes for Rational Team Concert is strongly recommended.

The RTC clients for Eclipse and Microsoft Visual Studio have received multiple enhancements in 4.0.1 under plan item 218176. Have a look at the videos below! Change review is now done in a new Change Summary view. This view is largely equivalent to the old Change Explorer when used to review changes, with additional features such as the Details Pane. The Change Explorer is now primarily used to compare workspaces and streams. Each view is optimized for their respective workflows (change review versus comparison of workspaces and streams).

Video: Efficiently and safely review code with the new Change Summary view in RTC Client for Microsoft Visual Studio

Video: Efficiently and safely review code with the new Change Summary view in RTC Eclipse Client

When reviewing changes, it’s important to know the things you don’t know – to the extent possible.

With the Change Summary view, the reviewer is clearly be made aware whether they can conduct a full review or if someone else needs to do it due to access rights restrictions on certain components, files or folders.
The reviewer will also be able to efficiently and safely review aggregated changes to a file when the corresponding change sets build on top of each other. When they don’t, the view will direct the reviewer toward the Details pane to review contributions from each change set individually, with a dependency graph explaining how the change sets reviewed depend on each other (maybe a change set is missing and it isn’t a good idea to approve the review).

The word change set is repeated many times here. Change set under the microscope will give you a detailed picture of what a change set is about.

With RTC, your team has the freedom to conduct peer reviews as they see fit, or setup the strictest process rules to govern every aspect of it. Chances are you will aim for something in the middle, something that works best for your team and the industry you are in.

(7 votes, average: 5.00 out of 5)

14 Comments

Rosa Naranjo October 12, 2012 @ 4:37 pm

Great article Christophe!
Christophe Cornu October 15, 2012 @ 8:53 am

Thanks Rosa :-)
May Wong October 29, 2012 @ 1:51 pm

Well done!Thanks for sharing, Christophe!
Manoj Panda December 11, 2012 @ 10:47 am

Hi Christophe,
Thanks. This is what most of the customers looking for but In this above example can Tim write some comments while doing the code review? what could be best practice here. As I know the review comments can be written inside the work item (may be in a comments by clicking on add comments)
but again is there any indication like which lines of the code Tim is commenting etc…

I feel the the complete life cycle of the Code review should be:-

user1 requested for Code review to user2 by using Approval functionality
User2 rejects the WI and make some comments (how??)
User1 check those comments from user2 and re-write the code.
finally user2 approves.
Christophe Cornu December 12, 2012 @ 12:01 pm

Here are two things we do today in our team.

1. When rejecting, we add a comment in the work item such as:
Foo.java
[copy a line from the compare editor and paste here]
[add a comment like “this line doesn’t check for null, it should”

It’s free format and it requires you copy and paste stuff from the compare editor.

2. For changes affecting the UI, the developer asking for an approval or review takes a screenshot of the UI before and after the fix (screen capture from within the work item editor). This makes it very easy for the reviewers to understand the visual change by simply hovering over the link to the screenshot in the work item comment. e.g. see comment 6 in https://jazz.net/jazz/web/projects/Rational%20Team%20Concert#action=com.ibm.team.workitem.viewWorkItem&id=188495 . It was easy for the approvers to understand which UI actions were added and understand the scope of the changes.

But you’re referring to the source code review workflow that allows you to annotate in place, on the diffs you are reviewing. You can give a try to third party tools such as Code Collaborator, which I’ve heard provide an integration with RTC (I haven’t tried it) – http://smartbear.com/products/software-development/code-review . You can also add yourself to the following enhancement request 82085 at https://jazz.net/jazz/resource/itemName/com.ibm.team.workitem.WorkItem/82085 and let us know you’re interested in this feature.

Hope this helps
Eltayeb Elzain January 22, 2013 @ 2:50 pm

Nice article – thanks
Udaya Puthuraya March 8, 2013 @ 6:09 pm

Seems with the introduction of Change Summary view in RTC 4x, when a file is opened in “Open in Comapre Editor” from Change Summary view, it does not open in External Comapre Tool, Even though in Preferences, Team -> Jazz Source Control -> External Compare Tools an extrenal tool is set as default for open action.
If a file is opened from Change Explorer view it seems to open fine with Extrenal compare editor.
Is there any extra setting for this or this is a bug ?
Tim Mok March 11, 2013 @ 9:40 am

https://jazz.net/jazz/resource/itemName/com.ibm.team.workitem.WorkItem/246125

The external compare support in Change Summary is a bug and has been fixed for 4.0.3 M2.
Jia Jia Li April 7, 2013 @ 2:41 am

It is a good way for code review. But if the leader approve it, could the leader deliver the change set directly? The second question, if the WI is a bug, when reopen it, the pre-condition of require approval recognize the first approval and will not require the approval again when deliver change set with the same WI, how to deal with it?
Christophe Cornu April 7, 2013 @ 4:48 pm

> could the leader deliver the change set directly?
No. Here’s what can be done: let’s assume leader has a repo workspace that is caught up with the stream to deliver to – his/her work environment is similar to the environment of the person who fixed the defect. Leader can right click on change set link in work item editor (links tab) > accept. This accepts the change set into their repo workspace. It shows as outgoing in Pending Changes view. Leader can verify things compile locally with the changes. Then leader can deliver change set to stream. However this requires the leader to have a workspace flowing to the intended stream (select stream in team artifact view > new repo workspace).

> how to deal with it?
https://jazz.net/library/article/1075/#recipe_gating_on_workitem_approvals

HTH
John Czukkermann January 16, 2014 @ 9:06 am

A little late to the party, but I have a couple observations. This approach works when one person is reviewing the code and providing comments without any kind of audit trail.

It breaks down with there are team reviews and there are multiple issues from multiple reviewers that need to be coordinated, shared and documented. In Eclipse, there are tools like Jupiter and a new one called AgileReview, but they really aren’t robust enough, and clearly aren’t integrated with Jazz.

When I do a code review, my ideal world would be something like this (Jazz centric of course).
Note: the review tooling CANNOT leave breadcrumbs in reviewed files that require committing of reviewed files by individual reviewers.

I develop my code and deliver it to my private workspace. I flow the change sets to a stream setup for code reviewing. I initiate a code review task in RTC and specify the stream from which reviewers can accept change sets into their own local workspaces for the purposes of reviewing.

As a reviewer, the review tooling will allow me to make the following kinds of comments.
1) Global comments – review scoped. That is, applying to all the code being reviewed
2) Global comments – file scoped.
3) Local comment – point in file. The comment applies to a particular line and space in the file.
4) Local comment – blocked. The comment applies to the selected block of code in the file.

The review tool must keep track of all comments made by the reviewer in such a way as to NOT add anything to the files being reviewed. The review tool must provide a way for me to make my comments available to the entire team of reviewers. Probably as a change set of some sort. Having the comments as a review file for each reviewer, which gets delivered to the review stream will probably be fine.

The tool must provide an intuitive, efficient way for other reviewers to navigate to and inspect any comment from any reviewer. It provides a global view of the current state of the review as reflected by the review stream.

The tool should be launchable from the review task and able to provide approval/disapproval in the review task when a reviewer is ready.

There should be a way for the person/people running the review to annotate comments/issues with resolutions. Additionally, the tool should provide the flexibility for review owners to define different classes of comments: issue, defect, minor, nit, etc. The tool should may provide a predefined set, but it should be completely configurable by the folks setting up the review.

Comments/issues/etc should be able to be flagged as completed, deferred, etc.
ian boden April 4, 2014 @ 10:09 am

I realize this is now quite a old article, but is there any information on opening a review such that you don’t have to open each individual file? Both the internal comparison tool, along with most external tools are perfectly happy opening directories for comparison rather than file, but so far I can’t find any option to do this.
Ulrich Kuhnhardt January 13, 2016 @ 11:12 pm

I’m missing inline code review commenting in RTC – pretty standard in most tools like Bitbucket, Atlassian Tools etc. Is there a solution to that?

Leaving a comment in the work item stating file and line number is not what I’m after.
楠姜 February 14, 2018 @ 8:43 pm

… [Trackback]

[…] There you will find 600 more Infos: jazz.net/blog/index.php/2012/10/11/reviewing-code-changes-with-rational-team-concert/ […]

You must be logged in to post a comment.

By Project

Tags

Authors

Archives

RSS Feeds