blob: 4610c950a47fcac45937c7a5f83f884bd489ace1 [file] [log] [blame]
This file has some general information about how 971 does code reviews.
We use Rietveld (<https://code.google.com/p/rietveld>) running in an app engine
app of ours. It is at <http://971code.appspot.com/>.
[General Procedure]
We try to code review all code before it gets checked in to svn. Code reviews
are also sometimes useful for getting feedback about documentation etc.
First, somebody starts a code review and sends out emails to people asking them
to look. Then, other people look at it and make suggestions (in the form of
comments). The person who started the review (the owner of it) looks at the
comments and responds to them and/or changes their code and uploads new
versions for everybody to look at. Once all of the reviewers approve the code,
the owner checks it in and closes the code review (issue).
[Reviewing]
This is the section for people who have received emails about reviewing code
should look.
The email that you receive will have a link to the issue (like
<http://971code.appspot.com/82001/>). Click on it to go to the issue page.
The most useful form of diff are the "Side-by-side diffs". Click the "View"
link in that column next to each file to look at it. After the first set of
comments, the "Delta from patch set" links are also helpful to see what got
changed so that you don't have to look at everything again. While looking at
a diff, double-click on any line of code to leave a comment there. You can
also reply to existing comments. The web interface sometimes hides comments
right after you create/edit them. Refresh the page to fix that (don't just do
it again, or you'll end up with 2 identical comments). Once you are done
looking at all of the files and making comments, click the
"Publish+Mail Comments" link (at the top of each diff or on the left of the
main issue page) to send out your comments. You can also put general notes in
the "Message" box. If you think that it looks good, then put "LGTM" at the top
of the message. The owner of the code review will keep looking at your
comments, making changes, and sending out more messages until it's finished.
[Starting a Review]
To begin, log in to <http://971code.appspot.com/> then click the "Create Issue"
link. Download the "upload.py" script and use that, not the web form, for
uploading a diff. (The web form has too many problems and is unusable with
git.) The script will find svn, git, or hg diffs in your current directory. If
you want the diff to cover back to an earlier change number, use the --rev
arg.
You don't need to give it a -s (--server) arg since the right default server
address is in the script. You can give it a -e (--email) arg with your gmail
address or let it ask you. (If you use 2-factor login for gmail, which I
recommend, you'll need to create an application-specific password instead of
using your regular password + OTP code.)
After it uploads, you can review the diffs on the web page. You can change
code and upload another diff. When ready, use the web UI to send the code
review email. Don't delete the old diffs; they are helpful for reviewers to
figure out what changed.
For more information about using Rietveld, see
<https://code.google.com/p/rietveld/wiki/CodeReviewHelp>.