Re: Need more reviewers!

On Thu, 2008-09-04 at 10:45 -0700, Josh Berkus wrote:

> We currently have 38 patches pending, and only nine people reviewing them.
> At this rate, the September commitfest will take three months.
>
> If you are a postgresql hacker at all, or even want to be one, we need your
> help reviewing patches! There are several "easy" patches in the list, so
> I can assign them to beginners.
>
> Please volunteer now!

Everybody is stuck in "I'm not good enough to do a full review". They're
right (myself included), so that just means we're organising it wrongly.
We can't expect to grow more supermen, but we probably can do more
teamwork and delegation.

I think this should be organised with different kinds of reviewer:

* submission review - is patch in standard form, does it apply, does it
have reasonable tests, docs etc.. Little or no knowledge of patch
required to do that. We have at least 50 people can do that.

* usability review - read what patch does. Do we want that? Do we
already have it? Does it follow SQL spec? Are there dangers? Have all
the bases been covered? We have 100s of people who can do that.

* feature test - does feature work as advertised?

* performance review - does the patch slow down simple tests? Does it
speed things up like it says? Does it slow down other things? We have at
least 100 people who can do that.

* coding review - does it follow standard code guidelines? Are there
portability issues? Will it work on Windows/BSD etc? Are there
sufficient comments?

* code review - does it do what it says, correctly?

* architecture review - is everything done in a way that fits together
coherently with other features/modules? are there interdependencies than
can cause problems?

* review review - did the reviewer cover all the things that kind of
reviewer is supposed to do?

If we split up things like this, we'll get a better response. Why not go
to -perform for performance testers, general for usability and feature
testers? If we encourage different types of review, more people will be
willing to step up to doing a small amount for the project. Lots of
eyeballs, not one good pair of eyes.

Also, why has the CommitFest page been hidden? Whats the point of that?

We probably need to offer some training and/or orientation for
prospective reviewers, so people understand what is expected of them,
how to do it and who to tell when they've done it.

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Training, Services and Support