Re: [9.4 CF 1] The Commitfest Slacker List

Szymon,

> I've reviewed some patches, but only some easier ones, like pure regression
> tests.

Actually, you were one of the people I was thinking of when I said
"mostly the new submitters have been exemplary in claiming some review
work". You're helping a lot.

> Unfortunately my knowledge is not enough to review patches making
> very deep internal changes, or some efficiency tweaks. I'm not sure if
> those patches should be reviewed by newbies like me, as I just don't know
> what is good and what is bad, even if a patch looks OK for me. What's the
> use of my review, if I don't understand the internals enough, I can apply
> the patch, run tests, look inside and I'm sure I won't find any problems?

Actually, something like 50% of all patches get sent back to the
submitter on the basis of a build/test/functionality check/completeness
check/standards compliance/do we really want this?. The fact that you
are doing those steps instead of a committer, and thus letting the
committer look at 50% fewer patches, *does* help.

In fact, the single most important part of the regression test reviews
is "does this new regression test test anything worthwhile?" and "does
this regression test return the same results on different machines?".
You already know enough to address both of those questions, at least
enough to bring up any potential problems on this list.

In the future, I would like to have automated systems do the
apply/build/regression test check, leaving new reviewers to check only
functionality and completeness and other things which require a human.
But we don't have the technology for that yet.

> Maybe this is the reason why there are not so many reviewers?
>
> I'm not sure if such a strict policy will bring anything good. If newbies
> won't be able to review patches, they won't be committing simple patches,
> as they won't be able to review others.

Commit a simple patch, review a simple patch. If we have 20 submitters
of simple patches, then we have 20 simple patches to review, no?

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com