ITK/Gerrit/ReviewPrimer

From KitwarePublic
< ITK‎ | Gerrit
Revision as of 14:09, 21 September 2010 by Blezek (talk | contribs) (→‎Review steps)
Jump to navigationJump to search

Gerrit code reviews can be relatively straightforward, but also give the reviewer a chance to look deep into proposed changes building and testing those changes locally. This primer is intended to guide the reader through the process of a deep review of a code change.

Prerequisites

This primer assumes a working knowledge of how to build ITK. No details will be given as to how to build and test the code (but they can be found here). This primer also assumes a working knowledge of git. Please see the ITK git pages and/or git books (ProGit, Community Book, or Version Control withGit).

Also required is a Gerrit change to review. We will go through a real life code review of a change submitted to Gerrit.

Select an open change

The primary review site for Gerrit/ITK is http://review.source.kitware.com. From this page, changes may be reviewed, etc. Our review will be of Change I5e76253f: Removed unecessary commented out debugging code commited on Sept. 17, 2010. Because Gerrit is dynamic, this primer will include many screenshots, YMMV.

The screen below is from Gerrit and contains several sections of interest.

A This section gives details of the change author (Hans Johnson), project(ITK), destination branch (master), topic (removeUnnecessaryComments), dates and status.
B This section is the commit message. The standard format for a commit is a single line summary, blank line, and longer description of the change. C This section gives the details of the reviewers. A developer may request certain registered developers as reviewers for the change, and any Gerrit registered user may comment. D This section gives details on the submitted code. It provides details on how to pull the changes from Gerrit. At the bottom is a list of files changed and links to diffs.

ReviewOverview.png

Review steps

  • Review the code in Gerrit
  • Use git to pull the changes
  • Build, test, review locally
  • Optionally, modify the commit and push changes back into Gerrit for review
  • Score the change with comments
  • If the change is ready, push it into the official repository

Review the code in Gerrit

The Gerrit developers have created a very useful system for code review that includes helpful shortcuts. The available shortcuts can be accessed by pressing the '?' key. ReviewShortcuts.png

From the change we are reviewing, press <enter> or "o" to open the diff viewer. You may also press the "Side-by-Side" link next to "Commit Message". This brings up the difference viewer.

ReviewCommitMessage.png

The first file to review is the commit message. At the top of the screen are some options controlling the display of the diffs. Change to suit your tastes.

Since there is not much to see, press ] to move to the next file:

ReviewNextFile.png

Lines 12-14 have been commented out in the change. Individual lines in the diff may be commented on by double clicking the line (when you are logged in to Gerrit). These comments are preserved in Gerrit and shown to the author and reviewers when the review is completed.

Let's comment on line 14:

ReviewComment.png

Clicking "Save" when finished:

ReviewSavedComment.png

Useful navigation commands

  • n Move to next diff within a file.
  • p Move to previous diff within a file.
  • ] Move to next file.
  • [ Move to previous file.
  • f Show/Hide list of files.

Use git to pull the changes

Press u or click "Up to changes" to return to the main change page.

Gerrit implements a full git server and provides hints as to how to pull this particular change. Be aware, that the hints are mainly command lines.

ReviewPatchSet.png