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

[Barry Warsaw]

On Sep 29, 2010, at 11:32 AM, Guido van Rossum 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.

I like and have used Rietveld, both as a submitter of a patch and as a
reviewer of someone else's code.  It's very nice, and I think we should use it
where appropriate, I don't think it should be a requirement.

While it will be somewhat better integrated into our normal development
processes whenever we move to Mercurial, it won't be seamless.  I don't
particularly like having to run a separate script (upload.py IIRC) in order to
initiate a review and push updates.  One thing I really like about Launchpad's
merge proposals is that it's very well integrated into normal workflows.
Updates against the target branch are automatically tracked in the generated
diff and in fact, once a merge proposal has been accepted, it can be
automatically landed by a 'bot if you want.

Launchpad's merge proposal system doesn't have the really nice web-based ui
that Rietveld has, but it is well integrated with an email-based workflow.
When I see a merge proposal come into my inbox, with the diff against the
target branch, I can just reply with my review inline right there, and those
comments are visible to all subscribers.  It lowers the barrier to performing
the review immensely.  Web is nice and should be available, but I really do
not want to give up on email-based reviews (well, with Python give up on the
possibility of them).

Someone else mentioned that it should better integrate with Roundup, and I
agree with that.

I would much rather we concentrate on converting over to Mercurial as soon as
possible, since I think a dvcs will do more to improve our processes than
anything else at this point.  Please, please, please, let's not let it wait
until Pycon 2011 (*2 years* after pronouncement) [1].

-Barry

[1] Apologies for sounding critical of any individual - that's not my intent.
Dirkjan and folks have done a lot of great work to this point and ISTM that
we're *really* close.  Let's JFDI and work out remaining kinks as we go!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://mail.python.org/pipermail/python-dev/attachments/20100929/f3f38234/attachment.pgp>