Re: next CommitFest - Mailing list pgsql-hackers

On Fri, 2009-11-13 at 10:55 -0500, Tom Lane 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.

I agree your point, but the principle is clear. Give back what you have
received. If somebody has submitted a complex patch then we expect them
to take a complex review. Once the principle has been established, we
will all follow it...and my point is...sponsors will then also be forced
to follow this also.

Dave may worry I am discussing his company. Actually, I'm not. I'm more
worried about my sponsors. If I am forced to do review, then I will have
to do it. If it is optional, then my sponsors will not pay for it. End
of story. Then we end up with a patch that is only reviewed if others
agree to do so. Sponsors don't win, but they can't see why.

I don't believe that you will personally fill the gaps in our review
process for ever and ever: that *would* be optimism. We need a system
that works in the longer term.

> Now when the current reviewing process was proposed, there were two
> separate goals in mind.  One was to take whatever incremental load
> we could off the eventual committer's work, by catching obvious
> mistakes, making sure the docs were up to snuff, etc.  The other was
> the idea that reviewing would in itself improve the skills of our
> development community, by making people read code that they wouldn't
> have read otherwise, and that eventually we'd have more committer-grade
> people just because of all the reviewing they'd done.  (The jury is
> still out on whether that will work, but in any case it's a long-term
> project.)
> 
> The problem at the moment seems to not be lack of first-level review
> time but lack of qualified review.  I don't know what we do about that.
> Requiring people who have submitted one or two patches to do reviews
> isn't going to produce more of the latter, it's going to produce more
> of the former.  Especially if the patches available to be reviewed
> are working in areas they haven't looked at before.

Review more, and we get better at it. Practice is the only way I know to
get better at something.

-- Simon Riggs           www.2ndQuadrant.com