Thread: Suggested new CF status: "Pending Discussion"

Folks,

I'd like to add a new CF status, "Pending Discussion".  This status
would be used for patches which have long discussions regarding syntax
or difficult functionality on this list which must be resolved before
commit.  Examples include SELECT INTO STRICT and Matviews WIP.

Currently these patches are kind of randomly listed as "waiting on
author", "waiting for review", or even "ready for commit".

Thoughts/objections/approval?

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

On 3/3/13 4:31 PM, Josh Berkus wrote:
> I'd like to add a new CF status, "Pending Discussion".  This status
> would be used for patches which have long discussions regarding syntax
> or difficult functionality on this list which must be resolved before
> commit.

I made a similar suggestion a few years ago.  Robert thought it was a 
workflow problem because it removed any notion of who was responsible 
for the next action.  Once something goes into "Discussion", it's easy 
to fall into a state where everyone is waiting for someone else.

I thought it was a useful idea anyway, but I could see his point.  This 
should probably move to "Waiting on Author" when it happens, presuming 
that the person who wrote something is motivated to see the change 
committed.  (If they weren't, why did they write it?)

-- 
Greg Smith   2ndQuadrant US    greg@2ndQuadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.com

> I thought it was a useful idea anyway, but I could see his point.  This
> should probably move to "Waiting on Author" when it happens, presuming
> that the person who wrote something is motivated to see the change
> committed.  (If they weren't, why did they write it?)

Except that the implication of "waiting on author" is that, if there's
no updates in a couple weeks, we bounce it.  And the author doesn't
necessarily control a bikeshedding discussion about syntax, for example.

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

On Sun, Mar 3, 2013 at 9:27 PM, Josh Berkus <josh@agliodbs.com> wrote:
>> I thought it was a useful idea anyway, but I could see his point.  This
>> should probably move to "Waiting on Author" when it happens, presuming
>> that the person who wrote something is motivated to see the change
>> committed.  (If they weren't, why did they write it?)
>
> Except that the implication of "waiting on author" is that, if there's
> no updates in a couple weeks, we bounce it.  And the author doesn't
> necessarily control a bikeshedding discussion about syntax, for example.

That's true.  I think, though, that the basic problem is that we've
lost track of the ostensible purpose of a CommitFest, which is to
commit the patches that *are already ready* for commit.  Very little
of the recently-committed stuff was ready to commit on January 15th,
or even close to it, and the percentage of what's left that falls into
that category is probably dropping steadily.  At this point, if
there's not a consensus on it, the correct status is "Returned with
Feedback".  Specifically, the feedback that we're not going to commit
it this CommitFest because we don't have consensus on it yet.

If we want to reopen development on 9.3 for another six months and
have a few more CommitFests, fine, we can decide to do that.  But if
we're NOT going to do that, then what we should be focusing our
attention at this point is looking at the things that haven't been
looked at yet that might be ready to go in - NOT bikeshedding on the
things that clearly aren't ready to go in but maybe with enough work
could be made so.

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

Robert Haas <robertmhaas@gmail.com> writes:
> On Sun, Mar 3, 2013 at 9:27 PM, Josh Berkus <josh@agliodbs.com> wrote:
>> Except that the implication of "waiting on author" is that, if there's
>> no updates in a couple weeks, we bounce it.  And the author doesn't
>> necessarily control a bikeshedding discussion about syntax, for example.

> That's true.  I think, though, that the basic problem is that we've
> lost track of the ostensible purpose of a CommitFest, which is to
> commit the patches that *are already ready* for commit.

Mumble.  That's *part* of the purpose of a CF, but not all.  It's also
meant to be a time when people concentrate on reviewing patches, and
surely discussions about syntax or whatever have to be part of that.

I recall in fact that at the last developer meeting, there was
discussion about trying to get people to do more formal reviewing of
design ideas that hadn't even made it to the submittable-patch stage.
So I feel it's counterproductive to try to narrow the concept of a CF
to "only ready to commit" patches.

But having said that, maybe the last CF of a cycle has to be treated
more nearly as you suggest.  Certainly if we hold off ending the CF
in hopes of committing stuff that wasn't nearly ready to commit at
its beginning, then we're back to bad old habits that seldom lead
to anything but a late release.
        regards, tom lane

On Mon, Mar 04, 2013 at 01:59:31PM -0500, Tom Lane wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
> > On Sun, Mar 3, 2013 at 9:27 PM, Josh Berkus <josh@agliodbs.com> wrote:
> >> Except that the implication of "waiting on author" is that, if there's
> >> no updates in a couple weeks, we bounce it.  And the author doesn't
> >> necessarily control a bikeshedding discussion about syntax, for example.
> 
> > That's true.  I think, though, that the basic problem is that we've
> > lost track of the ostensible purpose of a CommitFest, which is to
> > commit the patches that *are already ready* for commit.
> 
> Mumble.  That's *part* of the purpose of a CF, but not all.  It's also
> meant to be a time when people concentrate on reviewing patches, and
> surely discussions about syntax or whatever have to be part of that.

The distinction I recall arising from discussions about this time last year is
that we should review all submissions but only iterate within the CF on
patches that are about ready.  In other words, a process like this:

Review a patch in "Needs Review". Write up any problems in a reply to the submission. Problems are absent or trivial?
Mark patch Ready for Committer. Problems are few or isolated?      Mark patch Waiting on Author. Problems are many or
fundamental? Mark patch Returned with Feedback.
 

The current process typically looks more like this:

Review a patch in "Needs Review". Write up any problems in a reply to the submission. Problems are absent or trivial?
Mark patch Ready for Committer. CommitFest is too long overdue?    Mark patch Returned with Feedback. Else, mark patch
Waitingon Author.
 

(I've left out state transitions initiated by the patch author, rejection-type
decisions, and probably other things.)

-- 
Noah Misch
EnterpriseDB                                 http://www.enterprisedb.com

On Sun, Mar 3, 2013 at 9:27 PM, Josh Berkus <josh@agliodbs.com> wrote:
>> I thought it was a useful idea anyway, but I could see his point.  This
>> should probably move to "Waiting on Author" when it happens, presuming
>> that the person who wrote something is motivated to see the change
>> committed.  (If they weren't, why did they write it?)
>
> Except that the implication of "waiting on author" is that, if there's
> no updates in a couple weeks, we bounce it.  And the author doesn't
> necessarily control a bikeshedding discussion about syntax, for example.

That's true.  I think, though, that the basic problem is that we've
lost track of the ostensible purpose of a CommitFest, which is to
commit the patches that *are already ready* for commit.

 Very little
of the recently-committed stuff was ready to commit on January 15th,
or even close to it, and the percentage of what's left that falls into
that category is probably dropping steadily.  At this point, if
there's not a consensus on it, the correct status is "Returned with
Feedback".  Specifically, the feedback that we're not going to commit
it this CommitFest because we don't have consensus on it yet.

On Mon, Mar 4, 2013 at 11:50 PM, Jeff Janes <jeff.janes@gmail.com> wrote:
> Is that true of all commitfests, or only the last one in a cycle?  If the
> former, I think the existence of the "waiting on author" category belies
> this point.

The original point of "Waiting on Author" was that a patch might need
minor adjustments before it could get committed.  And, generally, for
the first few CommitFests, that's what happens.  But in the final
CommitFest, some people's notion of what a tweak in expands to include
multiple complete rewrites.  If a patch is basically OK, or only minor
points are being discussed, it's appropriate to punt it back to the
author and say, hey, fix these things and then we can commit this.
But if the whole method of operation of the patch needs rethinking,
then, in my opinion anyway, it should get pushed out to the last
CommitFest.

As for the last CommitFest, it's less that we should apply a different
standard and more that people fight back against the same standard a
lot harder when it means waiting a whole release cycle.

>>  Very little
>> of the recently-committed stuff was ready to commit on January 15th,
>> or even close to it, and the percentage of what's left that falls into
>> that category is probably dropping steadily.  At this point, if
>> there's not a consensus on it, the correct status is "Returned with
>> Feedback".  Specifically, the feedback that we're not going to commit
>> it this CommitFest because we don't have consensus on it yet.
>
> That is a fair point, and I think Tom has said something similar.  But it
> leaves open the question of who it is that is supposed to be implementing
> it.  Is it the commit-fest manager who decides there is not sufficient
> consensus, or the author, or a self-assigned reviewer?

It can be any of those.

> I know that I certainly would not rush into an ongoing a conversation, in
> which several of the participants have their commit-bits, and say "I'm
> calling myself the reviewer and am calling it dead, please stop discussing
> this."  Or even just, "stop discussing it as an item for 9.3".

Perhaps not, but you could certainly express an opinion, if you had
one.  If you express well-thought-out opinions a sufficient number of
times, the idea is that (along with various other things) lead to you
getting a commit bit, too.

> I think the role of the commit-fest manager is that of a traffic-cop, not a
> magistrate.  But if we are going to have "Commitfest II: The summary
> judgement", that needs to be run by a magistrate, as a separate process from
> the ordinary part of a commitfest.

I found that when I could spend 20 or 30 hours a week just managing
the CommitFest, I could do a fairly good job separating the stuff that
had a real chance of being ready with a modest amount of work from the
stuff that didn't.  But that basically involved me understanding, in a
fair degree of detail, every patch in the CommitFest.  Once I got my
commit bit, that became a lot less practical, because understanding
every patch in the CommitFest broadly and some of them well enough to
commit them added up to more work than I could do.  Also, that style
of CommitFest management involved me spending a lot of time arguing
with people who didn't want their patch punted no matter how
well-considered the arguments, and the novelty of that eventually wore
off.  To be fair, many people were very reasonable about the whole
thing - but the holdouts could suck up a huge amount of time and
emotional energy.

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

On 4 March 2013 18:59, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Sun, Mar 3, 2013 at 9:27 PM, Josh Berkus <josh@agliodbs.com> wrote:
>>> Except that the implication of "waiting on author" is that, if there's
>>> no updates in a couple weeks, we bounce it.  And the author doesn't
>>> necessarily control a bikeshedding discussion about syntax, for example.
>
>> That's true.  I think, though, that the basic problem is that we've
>> lost track of the ostensible purpose of a CommitFest, which is to
>> commit the patches that *are already ready* for commit.
>
> Mumble.  That's *part* of the purpose of a CF, but not all.  It's also
> meant to be a time when people concentrate on reviewing patches, and
> surely discussions about syntax or whatever have to be part of that.
>
> I recall in fact that at the last developer meeting, there was
> discussion about trying to get people to do more formal reviewing of
> design ideas that hadn't even made it to the submittable-patch stage.
> So I feel it's counterproductive to try to narrow the concept of a CF
> to "only ready to commit" patches.

Agreed.

"Ready to commit" is a state, and everybody has a different view of
the state of each patch. So we need a point where we all sync up and
decide by some mechanism what the group's opinion of the state is and
handle accordingly.

Or put another way, I very much welcome the chance for feedback from
others on my work, and appreciate the opportunity for review of others
work. If we can commit at any time, then every discussion needs to be
followed in detail otherwise we get "you should have said that
earlier" repeatedly. By all stopping, having a cup of tea and deciding
what we're happy with and then continue, it gives the opportunity for
efficient thread scheduling of our work. Between CFs we can work
mostly independently.

People starting new discussions while we have a big patch queue is
annoying, because it just makes the whole queue slow down and even
less gets done in the long run. We need to look at good overall
thruput, not continually interrupt each other, causing single
threading and overall loss of efficiency. Same idea as if we had a
meeting, it would be cool if everybody listened to the meeting, not
took calls and then walked back in and repeat discussions that already
happened. And overall, one long rolling discussion on hackers is the
same thing as saying all developers spend all day every day in a
meeting - we should recognise how unproductive that is and work out
ways to avoid it.

Process changes every year because the skills, capacities and
requirements change each year. So change doesn't imply last thing was
broken.

The general question is how can we work efficiently together and still
produce high quality software?


> But having said that, maybe the last CF of a cycle has to be treated
> more nearly as you suggest.  Certainly if we hold off ending the CF
> in hopes of committing stuff that wasn't nearly ready to commit at
> its beginning, then we're back to bad old habits that seldom lead
> to anything but a late release.

-- Simon Riggs                   http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services