[Python-Dev] Anyone can do patch reviews

[Tres Seaver]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

R. David Murray wrote:
> 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

This is an excellent set of guidelines.  The only drawback I see here is
that the current VCS situation makes doing the review more tedious than
it should be, especially for non-committers.  Or maybe the Hg mirrors
are truly up-to-date and working?  Last I looked, they were lagging or
unavailable.

> 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.



Tres.
- --
===================================================================
Tres Seaver          +1 540-429-0999          tseaver at palladion.com
Palladion Software   "Excellence by Design"    http://palladion.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkvXLtkACgkQ+gerLs4ltQ79DACbB35/XFGyiYjd79OtTx+kgoNl
mcsAnA4TNlM1ARjyrDrQIwv4KG48w/7h
=1hGI
-----END PGP SIGNATURE-----