|
|
- For background information on this project, see CodeReviewBackground.
- For known bugs, see KnownBugs.
- To contribute code, see ForContributors.
Starting a Code Review
A patch set is a set of diffs relative to a specfic revision of the files in a particular Subversion repository. The format used is a unified diff as output by the svn diff command.
There are three ways to start a code review:
1. Upload a patch set to the web app using the Create Issue form.
2. Let the web app fetch the patch set from another web server, for example the Python issue tracker. This is also done via the Create Issue form, using the "Url" field.
3. Use a small command line utility, upload.py, which you should install in your personal bin directory with mode 755 (it requires Python 2.5). This is by far the most convenient and versatile approach; the utility runs svn info and svn diff for you, and all you have to do is enter a brief message to be used as the subject line. The utility prints the URL for your newly created issue.
Inviting a Reviewer
To invite a reviewer for a particular issue, go to the page with details about the issue, and follow the "Publish+Mail Comments" link. This brings you to a simple form where you can add the reviewer's email address to the "Reviewers" field and enter a brief message explaining to them the purpose of the review. Clicking on "Publish All My Drafts" (the odd label will become clear later) will then send them an email. You can invite multiple reviewers by entering their email addresses separated by commas. Only the "user@domain.suffix" part of the email must be entered; do not enter the real name.
Conducting a Review
If you're invited to a review, you'll follow the link in the email, which takes you to the issue detail page. On that page you'll see a list of files. Selecting the filename will take you to a copy of the unified diff (not too interesting); following the link labeled "N comments" (it will probably say "0 comments" when you start the review :-) however takes you to a double source listing, a so-called side-by-side diff where the old version of the file is on the left and the new version is on the right. Deletions on the left are shown with a red background, additions on the right with a green background.
Navigating the Diff
In order to jump straight to the first diff chunk, hit the 'n' key once or twice. Each time you hit 'n' you are taken to the next chunk, until you have reached the end of the file. You can also use 'p' to move back up. Later, once there are in-line comments, these commands also stop at comments; use 'N' and 'P' to jump between comments only.
Long stretches of matching lines are suppressed; you'll see the text (...skipping N matching lines...) on a light blue background. this is done to avoid exceeding the strict limits enforced by App Engine on response size (1 MB) and request processing time (10 seconds).
If the patch set consists of multiple files, you can navigate between files using 'j' (next file) and 'k' (previous file). (Vi, Vim or Gmail users can think of these commands moving up ('k) or down ('j') in the list of files that you saw earlier.)
Entering In-line Comments
When you see something in the code you want to comment on, simply double-click the line on which you want to comment. Through the magic of JavaScript, a small text editing dialog will open up below that line, where you can enter as much text as you want. (There's a small '+' icon to the right that makes the text area larger each time you click it.) When you're done with a comment, don't forget to click the "Save" button (hitting Control-s works too). If you clicked "Save" too soon or change your mind, you can always hit the "Edit" link that appears under the completed comment, and edit or discard it.
Publishing In-line Comments
You're not done yet! Once you have splattered comments all over the files in the patch set, there's one more thing to do: you must publish your comments! So far, all your comments have been stored in the database as drafts, which means that only you can see and edit them. To remind you of this "limbo" state, the issue page lists the number of draft comments in red, and displays a pretty alarming warning at the top when you have any draft comments. To get rid of this, follow one of the "Publish+Main Comments" links (there are several on the diff page as well as on the issue page), enter a brief message, and click "Publish All My Drafts". This will send an email to the issue owner and to all reviewers, and publish your drafts on the web app so that others can see them. (If you'd rather not send email, you can uncheck the "Send mail" check box.)
Access Control
Apart from draft comments, which are only visible to their author, all comments are visible to anyone who visits the site, even if they are not logged in. Anybody can view all issues, too. Only the owner of an issue can edit its subject, description, and list of reviewers, but anybody (even people not listed as reviewers) can add new comments to an issue. Comments however cannot be entered anonymously; you must be logged in.
Known Bugs
See the KnownBugs page.
Sign in to add a comment
