On Fri, Nov 13, 2009 at 10:55 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Simon Riggs <simon@2ndQuadrant.com> writes:
>> All the CF manager needs to do is ensure that every patch submitted
>> chalks up one review. If you think about it, we wouldn't actually need
>> any rr reviewers at all then, because if we have 20 patches we would
>> have 20 reviews due. So the whole scheme is self-balancing.
>
> Well, no, that's *far* too optimistic/simplistic, because it imagines
> that every review is worth the same. What we lack is not just review
> time but qualified review time, ie, comments from someone who's already
> familiar with the portion of the code base that's being patched.
Right, but I think we're more likely to find such people among the
pool of existing contributors than we are among people who don't write
patches themselves but happen to volunteer to review.
I think Simon's idea of requiring 1 review per patch probably IS a bit
overly simplistic - for one thing, someone who submits 10 patches, as
I did in the July CommitFest, can scarcely be expected to also review
10 patches. (Even if they were willing, it would make the CommitFest
longer, not shorter.) But I don't think they should get by reviewing
none, either, especially if they're submitting patches to every
CommitFest.
It's not my idea that we should punish someone like Dave Page who does
a lot of PostgreSQL work and occasionally writes a patch. What I'm
complaining about is people who submit patches regularly and rarely or
never review. We have enough volunteers to cover new and occasional
patch submitters; sometimes those reviews are not quite as thorough,
but new and occasional contributors tend to submit relatively simple
patches anyway, so it's not a catastrophe. It's the regular patch
submitters who, IMHO, most need to be involved.
...Robert