Horton

Thread

  1. No answers on CommitFest procedures?

    Josh Berkus <josh@agliodbs.com> — 2008-07-09T17:58:11Z

    Folks,

Have received exactly zero feedback on the question of whether I should 
be assigning reviewers to "WIP" patches or not.

--Josh

  2. Re: No answers on CommitFest procedures?

    Pavan Deolasee <pavan.deolasee@gmail.com> — 2008-07-09T18:11:32Z

    On Wed, Jul 9, 2008 at 11:28 PM, Josh Berkus <josh@agliodbs.com> wrote:
> Folks,
>
> Have received exactly zero feedback on the question of whether I should be
> assigning reviewers to "WIP" patches or not.
>

I think we should. For example, one of the WIP patches is submitted by
me. One reason I marked it WIP because I was not sure what is
considered as WIP and what is not. The patch is ready for review, but
of course have some open items and may need further work once I
receive the feedback. But I can not make progress either before the
current work is reviewed, either at the code level or design level. I
expect that feedback during this commit fest.

Thanks,
Pavan

-- 
Pavan Deolasee
EnterpriseDB http://www.enterprisedb.com

  3. Re: No answers on CommitFest procedures?

    Neil Conway <neilc@samurai.com> — 2008-07-09T18:24:51Z

    On Wed, 2008-07-09 at 10:58 -0700, Josh Berkus wrote:
> Have received exactly zero feedback on the question of whether I should 
> be assigning reviewers to "WIP" patches or not.

I suppose it depends on what exactly "WIP" means, but I should think
that if work is still "in progress", the patch is not ready to be
reviewed yet. Patches that are truly WIP probably shouldn't be
candidates for a commit fest until they are finished, anyway.

BTW, the pg_dump lock timeout patch was previously listed as "WIP", but
I think it is better classified as "waiting for response". Waiting for a
reviewer's comments to be incorporated into a new version of the patch
should be a distinct state from waiting for the initial version of the
patch to be completed.

-Neil

  4. Re: No answers on CommitFest procedures?

    Josh Berkus <josh@agliodbs.com> — 2008-07-09T18:30:19Z

    Pavan,

> I think we should. For example, one of the WIP patches is submitted by
> me. One reason I marked it WIP because I was not sure what is
> considered as WIP and what is not. The patch is ready for review, but
> of course have some open items and may need further work once I
> receive the feedback. But I can not make progress either before the
> current work is reviewed, either at the code level or design level. I
> expect that feedback during this commit fest.

I guess the problem I'm having is that people are using various statuses, 
but there's no clear indication what they mean.  Currently on the 
commitfest we have:

Pending Review
WIP 
Waiting on Response
Proof of Concept

And for the non-committer reviewers, we'll probably need "Ready for 
Committer" as well.  I thing we need to restrict the list of statuses to a 
coherent list, and have definitions for each.

-- 
--Josh

Josh Berkus
PostgreSQL @ Sun
San Francisco

  5. Re: No answers on CommitFest procedures?

    Alvaro Herrera <alvherre@commandprompt.com> — 2008-07-09T18:38:54Z

    Neil Conway wrote:
> On Wed, 2008-07-09 at 10:58 -0700, Josh Berkus wrote:
> > Have received exactly zero feedback on the question of whether I should 
> > be assigning reviewers to "WIP" patches or not.
> 
> I suppose it depends on what exactly "WIP" means, but I should think
> that if work is still "in progress", the patch is not ready to be
> reviewed yet. Patches that are truly WIP probably shouldn't be
> candidates for a commit fest until they are finished, anyway.

But patches in progress still need comments from reviewers.  Either the
developer needs to change direction, or he needs someone to comment on
what the future direction needs to be.

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

  6. Re: No answers on CommitFest procedures?

    Neil Conway <neilc@samurai.com> — 2008-07-09T18:42:54Z

    On Wed, 2008-07-09 at 14:38 -0400, Alvaro Herrera wrote:
> But patches in progress still need comments from reviewers.

Certainly, but should this be done as part of the commit fest process?
Commenting on the future direction that an in-development patch ought to
take sounds more like a task for -hackers at large, rather than for an
individual reviewer.

-Neil

  7. Re: No answers on CommitFest procedures?

    Alvaro Herrera <alvherre@commandprompt.com> — 2008-07-09T18:47:09Z

    Neil Conway wrote:
> On Wed, 2008-07-09 at 14:38 -0400, Alvaro Herrera wrote:
> > But patches in progress still need comments from reviewers.
> 
> Certainly, but should this be done as part of the commit fest process?
> Commenting on the future direction that an in-development patch ought to
> take sounds more like a task for -hackers at large, rather than for an
> individual reviewer.

Well, we can discuss that of course :-) but keep in mind that the point
of having commitfests at all was so that we could have promises about
patches in progress, not just things that are "ready for commit".

For example, HOT was posted to -hackers several times but still there
was almost no review of it until very late in the cycle; which is
exactly what we want to avoid.

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

  8. Re: No answers on CommitFest procedures?

    Gregory Stark <stark@enterprisedb.com> — 2008-07-09T20:30:13Z

    "Neil Conway" <neilc@samurai.com> writes:

> On Wed, 2008-07-09 at 14:38 -0400, Alvaro Herrera wrote:
>> But patches in progress still need comments from reviewers.
>
> Certainly, but should this be done as part of the commit fest process?
> Commenting on the future direction that an in-development patch ought to
> take sounds more like a task for -hackers at large, rather than for an
> individual reviewer.

This is, from my point of view as a patch submitter, the *whole* *point* of
the commitfests. For 8.3 We had lots of people submit patches, then sit around
for months waiting for feedback. By the time they got feedback the patches had
bitrotten and it was feature-freeze time and there was a lot of pressure to
finish patches quickly.

If the author isn't looking for feedback the patch wouldn't be in the commit
fest queue.

"WIP" just means the submitter knows the patch isn't ready to commit yet.
They're probably stuck on a major design decision or just want confirmation
that they're on the right track before they invest months more of work.

I used "proof of concept" for one of my patches just to indicate it was just a
quick hack to demonstrate an idea. In this case the implementation details are
not so relevant as just whether we like the idea at all.

-- 
  Gregory Stark
  EnterpriseDB          http://www.enterprisedb.com
  Ask me about EnterpriseDB's Slony Replication support!