[Python-Dev] We should be using a tool for code reviews

Guido van Rossum guido at python.org
Wed Sep 29 20:49:31 CEST 2010


On Wed, Sep 29, 2010 at 11:41 AM, Antoine Pitrou <solipsis at pitrou.net> wrote:
> On Wed, 29 Sep 2010 11:32:19 -0700
> Guido van Rossum <guido at python.org> wrote:
>> I would like to recommend that the Python core developers start using
>> a code review tool such as Rietveld or Reviewboard. I don't really
>> care which tool we use (I'm sure there are plenty of pros and cons to
>> each) but I do think we should get out of the stone age and start
>> using a tool for the majority of our code reviews.
>
> He, several of us would like it too (although for short patches it
> doesn't really make a difference), but what's missing is some kind of
> Roundup integration. Something as trivial as a "start review" button in
> front of every uploaded patch file would do the trick; it has been
> suggested several times already, but what's needed is someone to write
> the code :)

As I tried to explain in the Buzz thread (not that I require you to
read it -- I'll happily repeat myself here):

Unfortunately taking the average patch posted to the tracker and
importing it in Rietveld is very iffy -- it's very hard to find the
right branch+rev needed to be able to apply the patch correctly -- not
to mention that there are so many (slightly) different patch formats.
It would take a fair bit of AI to get this right.

The recommended way to work with Rietveld is to use a checkout
(fortunately with Hg this will become easier to do) and use the
"upload.py" script that you can download from Rietveld (log in and
click on the Create link).

Other projects that have adopted Rietveld (Chromum, GWT, Go) require
its use for all reviews and have written their own scripts extending
upload.py to implement the workflow they like (each one a bit
different). They've also created their own branded Rietveld sites
(which is easy to do using App Engine).

-- 
--Guido van Rossum (python.org/~guido)


More information about the Python-Dev mailing list