[Python-Dev] Define a place for code review in Python workflow

[anatoly techtonik]

http://bugs.python.org/issue9376
This issue discussed docs on the proper way to create diff on windows
(as it is doesn't have the tool) for sending the patch. The current
proper way is to use "svn diff" which will be replaced with "hg diff".

I proposed using Rietveld like:
> python -m easy_install review
> python -m review

But Brian said using Rietveld at all is not a good idea, and didn't
want to answer what should be done for it to become good idea.

Probably Brian is too busy, that's why I'd like to ask people here.
What do you need or expect to happen to start using code review in
Python workflow?


== Intro ==
Small introduction for insiders not familiar with outsider's problem
of maintaining patches in tracker. Please forgive the tone I write
about things I dislike, but I am not devoting my life to earn a title
of polite bastard (this one is obligatory disclaimer I find it still
doesn't work for all people, so expect some irrelevant flame in
follow-ups).

Ok, for an outsider like me "svn diff" 'best practice' suxx greatly,
because in non-ideal conditions (and it is about 90% of cases) patches
are often needed to be edited, and reuploaded. There are usually too
few insiders to review you patch, and they are usually too busy to
edit a small typo, and they also deny that they need an online tool to
see if patch applies clearly, so if your patch doesn't apply clearly
you will be asked to check where the problem is. The "svn diff" upload
story continues time after time.

The problem of too few insiders is that there are too few of them, and
in case your patch is somehow complicated it can enjoy some years of
loneliness. During this time not only the code can change, but the
codebase itself can move to some other place.

Some of the few are devoting great time to review new contributions
and everything could be fine, but.. there is still a big time lapse,
big enough that your patches start to overlap.. Trying to get out of
this maintenance nightmare you will start learning Mercurial, then
Mercurial Queues, then you go some weeks practicing or.. you will just
give up right away, because time is more valuable than money.

If you're an insider, you can propose to review patches in ViewVC, but
it is just plain wrong. Just because. Just because there are tools
that do the job better. Even if they have awful usability interface.
That can be improved though. By stealing templates from Gerrit.


== How Rietveld helps to save time ==
Assuming that Python has "easy_install" packaging solution bundled by
default (which it doesn't of course):
> python -m easy_install review

This is run once to install the Rietveld script that is used from
command like like:

> python -m review

and allows you to:

1. Create issue for patch review on Rietveld site
2. Run "svn diff"
3. Upload the patch
4. Supply comment for the patch

everything above in one step. To upload an updated patch, you just do:

> python -m review -i XXXXX -m "log of fixes made in comparison with previous patch"

Everybody can go to Rietveld site to view either patch or the whole
file code _with_ the patch. Everybody can add comments _directly_ near
patch lines.
Everybody receives notifications about new code review comments.


As code comments are hard to keep offtopic, the signal to noise ration
appears to be is quite high.
The patch upload script is designed in a way that every project can
tune it for their needs and place into the root of source code.
"review" project at PyPI.is uploaded from source at
http://bitbucket.org/techtonik/pypi-rietveld - it can be tuned to
address needs specific for Python development.


What do you need to start using code review for contributed Python patches?
Why you wouldn't recommend this practice to outsiders?

-- 
anatoly t.