[Python-Dev] Anyone can do patch reviews (was: Enhanced tracker privileges...)

[R. David Murray]

On Tue, 27 Apr 2010 11:15:49 +1000, Steven D'Aprano <steve at pearwood.info> wrote:
> No, of course not. There are always other reasons, the biggest is too 
> many things to do and not enough time to do it. If I did review 
> patches, would they be accepted on the strength on my untrusted 
> reviews?

It is very very helpful for *anyone* to review patches.   Let's see if
I can clarify the process a little.  (This is, of course, my take
on it, others can chime in if they think I got anything wrong.)

Someone submits a bug.  Someone submits a patch to fix that bug (or add
the enhancement).  Is that patch ready for commit?  No.  Is it ready
for *commit review* (ie: someone with commit privileges to look at it
with an eye toward committing it)?  Probably not.

What makes a patch ready for commit review?  The patch should:

    1) conform to pep 7/8
    2) have unit tests that fail before the patch and succeed after
    3) have documentation updates if needed
    4) have a py3k port *if and only if* the port is non-trivial
        (well, if someone wants to add one when it is trivial that's OK,
        but it probably won't get used)
    5) if it is at all likely to have system dependencies, be tested
        on at least linux and windows

Making sure that these items are true does not require any in-depth
expertise. In many cases it doesn't even require much time.  'Trusted'
or 'untrusted' doesn't really come in to it, though doing these sorts
of reviews will build trust.  If you can in addition look at the patch
content and critique it, so much the better.  Again, *any* critique is
useful, even if you can't review the whole patch in detail, because it
gets it that much closer to being commit ready.  And there are enough
uncommitted patches in the tracker that it ought to be possible for almost
anyone to find something they can usefully do a content critique on.

The goal is to make the commit review step as simple and fast for the
committer as possible.  The more eyes on the patch before hand, the
faster the commit review will be.  And those people who do a good job
making patches commit ready will be on the fast track to getting commit
privileges.

--
R. David Murray                                      www.bitdance.com

PS: note that I'm using 'commit review' above with a different sense than
that value is currently defined to have in the workflow.  I'm thinking
about advocating that the definition in the workflow be changed, and
indeed we (the informal triage crew) have already occasionally used that
setting with the meaning I give it above.