Re: new CommitFest states (was: YAML) - Mailing list pgsql-hackers

On Mon, Dec 7, 2009 at 10:20 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Itagaki Takahiro <itagaki.takahiro@oss.ntt.co.jp> writes:
>> Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> It was written and submitted by one person who did not bother to ask
>>> first whether anyone else thought it was worthwhile.  So its presence
>>> on the CF list should not be taken as evidence that there's consensus
>>> for it.
>
>> Should we have "Needs Discussion" phase before "Needs Review" ?
>> Reviews, including me, think patches with needs-review status are
>> worthwhile. In contrast, contributers often register their patches
>> to CF without discussions just because of no response; they cannot
>> find whether no response is silent approval or not.
>
> Hm, I guess the question would be: what is the condition for getting
> out of that state?  It's clear who is supposed to move a patch out of
> 'Needs Review', 'Waiting for Author', or 'Ready for Committer'
> respectively.  I don't know who's got the authority to decide that
> something has or has not achieved community consensus.
>
> Right at the moment we handle this sort of problem in a very informal
> way, but if it's going to become part of the commitfest state for a
> patch I think we need to be a bit less informal.

This sounds suspiciously like it could lead to a situation where
people want to use the CommitFest application to track feature ideas
rather than actual patches.  The next step would be to require that
the "Needs Discussion" status doesn't actually require any code, and
the code can be submitted when it reaches "Needs Review".  I think
this whole line of thinking is a bad one.  The CommitFest application
isn't designed to manage feature requests, and I don't want to turn it
into something that does.  We might need a system to manage that, at
some point, but let's not shoehorn it into what we have now.

The right answer here is that no one should assume that a patch is
wanted just because it is in the CommitFest application.  This point
is actually explicitly addressed in this document

http://wiki.postgresql.org/wiki/Reviewing_a_Patch

If people submit patches without getting consensus that the feature is
something that we want, then the penalty is that their patch is more
likely to be rejected.  If you're worried about this, then you should
get consensus before you spend time writing code.  If you're not
worried about it, that's your prerogative: experienced patch authors
often know what will and will not fly without a drawn-out discussion,
especially for very minor enhancements or bug fixes.

On a related note, Greg Smith requested a state called "Discussing
Review", which would logically follow "Needs Review" and precede
"Waiting for Author"/"Ready for Committer"/"Returned with Feedback".
I'm not altogether convinced of the value of that state, but I'm not
altogether opposed to it either.  If we're going to have a discussion
of CommitFest states, we probably ought to talk about that one, too.

...Robert