Re: [GENERAL] [RRR] [HACKERS] Commitfest: The Good, The Bad, and the Ugly

2010-09-28 Thread Robert Haas
On Tue, Sep 28, 2010 at 9:33 PM, Itagaki Takahiro
 wrote:
> On Wed, Sep 29, 2010 at 10:18 AM, Robert Haas  wrote:
>> No, the column is very clearly labelled "Reviewers", not "Reviewer".
>> And we have certainly had patches with more than one person's name in
>> that field in the past.  The issue is rather that we don't have enough
>> people reviewing.  We haven't had enough people volunteer to do
>> reviews to even assign ONE person to each patch, let alone two.  There
>> are, as of this writing, SEVEN patches that have no reviewer at all.
>
> Some of them might be too difficult to review. For example, replication
> or snapshot management requires special skills to review.
>
> I'm worrying about new reviewers hesitate to review a patch that has
> a previous reviewer, and then, if they think the remaining patches are
> too difficult for them, they would just leave the commitfest page.

That's a legitimate concern, but I am not sure how much of a problem
it is in practice.  Most people who become round-robin reviewers are
getting pulled into the process a little more than just stumbling
across the CF page by happenstance, or at least I hope they are.  Not
all patches can benefit from multiple reviewers, but CF managers can
and should encourage multiple reviews of those that can.  However, at
the moment, the problem is that regardless of who is assigned to do
what, we're not getting enough reviews done.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

-- 
Sent via pgsql-general mailing list (pgsql-general@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-general


Re: [GENERAL] [RRR] [HACKERS] Commitfest: The Good, The Bad, and the Ugly

2010-09-28 Thread Andy Colson

On 9/28/2010 8:33 PM, Itagaki Takahiro wrote:

On Wed, Sep 29, 2010 at 10:18 AM, Robert Haas  wrote:

No, the column is very clearly labelled "Reviewers", not "Reviewer".
And we have certainly had patches with more than one person's name in
that field in the past.  The issue is rather that we don't have enough
people reviewing.  We haven't had enough people volunteer to do
reviews to even assign ONE person to each patch, let alone two.  There
are, as of this writing, SEVEN patches that have no reviewer at all.


Some of them might be too difficult to review. For example, replication
or snapshot management requires special skills to review.

I'm worrying about new reviewers hesitate to review a patch that has
a previous reviewer, and then, if they think the remaining patches are
too difficult for them, they would just leave the commitfest page.



If I might...  I think it would be good to have new reviewers teamed 
with experienced reviewer on a single patch.  Let the newbie have a 
crack at it while having a safety net too.  Good for the newbie, good 
for the project.


You just need a way to assign them.  Message's are already sent out 
saying "if you wanna help, email xyz to get started".  A message like 
that could be added to the web page.  The commitfest overlord could 
assign the newbie a patch and a sponsor, saying "dear newbie you'll be 
working with bob on this patch, ask him any questions about the process, 
you'll both do the review and you can compare your work to his, and 
learn the process along the way".


The sponsor could answer any dumb questions off list, and every once and 
a while say "Hey, that's a great thing to post to the mailing list for 
everyone to review".


And/Or/Also, maybe a snippet on the page saying "dont feel like you have 
to do a full review, if you can only do a part, do it, and someone else 
can do the rest."  And/Or: "feel free to review something someone else 
is reviewing, more eyes = better prize".


-Andy

--
Sent via pgsql-general mailing list (pgsql-general@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-general


Re: [GENERAL] [RRR] [HACKERS] Commitfest: The Good, The Bad, and the Ugly

2010-09-28 Thread Itagaki Takahiro
On Wed, Sep 29, 2010 at 10:18 AM, Robert Haas  wrote:
> No, the column is very clearly labelled "Reviewers", not "Reviewer".
> And we have certainly had patches with more than one person's name in
> that field in the past.  The issue is rather that we don't have enough
> people reviewing.  We haven't had enough people volunteer to do
> reviews to even assign ONE person to each patch, let alone two.  There
> are, as of this writing, SEVEN patches that have no reviewer at all.

Some of them might be too difficult to review. For example, replication
or snapshot management requires special skills to review.

I'm worrying about new reviewers hesitate to review a patch that has
a previous reviewer, and then, if they think the remaining patches are
too difficult for them, they would just leave the commitfest page.

-- 
Itagaki Takahiro

-- 
Sent via pgsql-general mailing list (pgsql-general@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-general


Re: [GENERAL] [RRR] [HACKERS] Commitfest: The Good, The Bad, and the Ugly

2010-09-28 Thread Robert Haas
On Tue, Sep 28, 2010 at 9:11 PM, Itagaki Takahiro
 wrote:
> On Wed, Sep 29, 2010 at 6:03 AM, David Fetter  wrote:
>> The Good:
>> - Most patches still in play have a reviewer.
>
> As far as I remember, there were discussions about the issue
> "A patch has a reviewer, but in Needs Review state for several weeks "
> in 9.0 development.
>
> Do we have any plans for it? According to the commitfest app, one patch
> has only one reviewer at once. A new reviewer might avoid reviewing
> a patch that have another reviewer already.

No, the column is very clearly labelled "Reviewers", not "Reviewer".
And we have certainly had patches with more than one person's name in
that field in the past.  The issue is rather that we don't have enough
people reviewing.  We haven't had enough people volunteer to do
reviews to even assign ONE person to each patch, let alone two.  There
are, as of this writing, SEVEN patches that have no reviewer at all.

Of course, several of the committers, including you, me, and Tom, have
been working our way through the patches.  And that is great.  But the
point of the CommitFest process is that everyone is supposed to pitch
in and help out, not just the committers.  That is not happening, and
it's a problem.  This process does not work and will not scale if the
committers are responsible for doing all the work on every patch from
beginning to end.  That has never worked, and the fact that we have a
few more committers now doesn't change that.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

-- 
Sent via pgsql-general mailing list (pgsql-general@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-general