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

Nick Coghlan ncoghlan at gmail.com
Wed Sep 29 23:45:52 CEST 2010


On Thu, Sep 30, 2010 at 4:47 AM, Brett Cannon <brett at python.org> wrote:
> On Wed, Sep 29, 2010 at 11:41, 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.

Somewhat amusing to get to this thread a few minutes after creating a
Reitveld issue for the first pass of my urllib.parse patch :)

>> 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 :)
>
> The other option (as discussed on Buzz) is to add Rietveld's upload.py
> to Misc/ and tell people to use that to submit the patch. Then we
> simply say to the person submitting the patch, "upload it to Rietveld
> and paste in the link" or simply require it upfront to encourage
> people to do the upload in the first place. This would let usage to
> move forward until we get that "start review" button (wasn't Ezio
> looking into it?).

Make our script a "submit_patch.py" wrapper around the vanilla
"upload.py" rather than a replacement of it and it sounds like a good
idea to me. I'd want to be able to do a signature check on upload.py
before we enabled runtime downloading of new versions from the
codereview site though (in the meantime, a vetted version checked into
SVN would do the trick).

Even with Roundup integration, being able to create/update the
Reitveld issue and make the appropriate tracker updates with a single
command would still be handy.

Cheers,
Nick.

-- 
Nick Coghlan   |   ncoghlan at gmail.com   |   Brisbane, Australia


More information about the Python-Dev mailing list