[Python-Dev] We should be using a tool for code reviews (original) (raw)

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