Re: Commitfest process

It's not clear how this commitfest thing is supposed to work in
practice. May I suggest that:

1. When a patch author wants to have a patch reviewed in the next
commitfest, he posts it to pgsql-patches as usual, and then adds it to
the list on the Todo:PatchStatus page (or perhaps even better, a new
page per commitfest with same layout) in the wiki himself, with status
"awaiting review".

2. When a patch is outright rejected, the patch author updates the
status accordingly.

3. Many patches will not be ready for committing yet, because there's
bugs that need fixing, or it needs performance testing or whatever. If
it's a quick thing, patch author can just submit an updated patch, or
test results or whatever and continue discussion. Otherwise, after
author knows what he's going to do next, he updates the status on the
wiki accordingly. The status can be something like "will do performance
testing", "will try approach suggested by X", "will clean up comments" etc.

4. The commitfest is over when there is no more tasks on the wiki page
with status "awaiting review".

The trick here is that the patch authors drive the process. An author
will not change the status of a patch until he knows what he is going to
do next. For example, you might get feedback from one person, but if you
feel like you want another opinion, you can keep the status at "awaiting
review" until you get that. (should reply on the mailing list with "what
do others think?" as well, of course)

It's also OK to submit design plans etc. non-patch items to the list, if
you want people to review them before you start writing a patch.

The commitfest will not go on forever because:

- Patch reviewers know where to look for patches to review, which makes
it easy for people to participate. The most active reviewers are also
most active patch authors, and they likely have a patch or two in the
list themselves, which they can't work on until the commitfest is over
(or at least that would be frowned upon). That's a good motivator to
review other people's patches.

- Patch authors don't want to hold up the commitfest, because after your
patch is one of the few ones left in the list, you will start to feel
the heat from people who want to move on, if you don't update the status.

I don't think we should have a reviewer field in the status page, BTW.

This will not help with items that no-one is actively working on, but at
least in my mind, the purpose of commitfests is to get attention to
patches people are working on, and need advice on how to proceed.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Fri, 07 Mar 2008 14:33:02 +0000
"Heikki Linnakangas" <heikki(at)enterprisedb(dot)com> wrote:

> It's not clear how this commitfest thing is supposed to work in
> practice. May I suggest that:
>
> 1. When a patch author wants to have a patch reviewed in the next
> commitfest, he posts it to pgsql-patches as usual, and then adds it
> to the list on the Todo:PatchStatus page (or perhaps even better, a
> new page per commitfest with same layout) in the wiki himself, with
> status "awaiting review".
>

This is a general workflow issue. You are asking patch submitters to do
double work, (exactly what a tracker should be doing but I digress). We
need to have a single point of entry. At this point I think the patch
list is defunct. Have a patch category on the wiki. Each patch is a
page. Each revision of the patch is uploaded to the page that is
assigned to the patch.

> 2. When a patch is outright rejected, the patch author updates the
> status accordingly.

I don't find this realistic. I believe we will just end up trolling
back through patch archives trying to remember what the status was.

>
> 3. Many patches will not be ready for committing yet, because there's
> bugs that need fixing, or it needs performance testing or whatever.
> If it's a quick thing, patch author can just submit an updated patch,
> or test results or whatever and continue discussion. Otherwise, after
> author knows what he's going to do next, he updates the status on the
> wiki accordingly. The status can be something like "will do
> performance testing", "will try approach suggested by X", "will clean
> up comments" etc.

I assume this happens "After" discussion on -hackers?

>
> 4. The commitfest is over when there is no more tasks on the wiki
> page with status "awaiting review".

Nod.

>
> The trick here is that the patch authors drive the process. An author

Yes and I believe it is a trick that is destined to bomb at the magic
show. We can't convince hackers to follow standard bug tracker policies
but we are going to convince them to keep a mailing list + wiki up to
date?

Please don't misunderstand I certainly thinking working about the kinks
of the commit fest are important. It is new to us but I don't think
adding multiple points of entry and multiple documentation paths is
going to help.

Sincerely,

Joshua D. Drake

- --
The PostgreSQL Company since 1997: http://www.commandprompt.com/
PostgreSQL Community Conference: http://www.postgresqlconference.org/
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
PostgreSQL SPI Liaison | SPI Director | PostgreSQL political pundit

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFH0YX3ATb/zqfZUUQRAgD4AJsFGgnuaVKbLe89xvdfzXm0AuuZRwCdFswJ
qm1cLFj8GySWaMNbco+Ydts=
=cj5Y
-----END PGP SIGNATURE-----

Joshua D. Drake wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On Fri, 07 Mar 2008 14:33:02 +0000
> "Heikki Linnakangas" <heikki(at)enterprisedb(dot)com> wrote:
>
>> It's not clear how this commitfest thing is supposed to work in
>> practice. May I suggest that:
>>
>> 1. When a patch author wants to have a patch reviewed in the next
>> commitfest, he posts it to pgsql-patches as usual, and then adds it
>> to the list on the Todo:PatchStatus page (or perhaps even better, a
>> new page per commitfest with same layout) in the wiki himself, with
>> status "awaiting review".
>>
>
> This is a general workflow issue. You are asking patch submitters to do
> double work, (exactly what a tracker should be doing but I digress). We
> need to have a single point of entry. At this point I think the patch
> list is defunct. Have a patch category on the wiki. Each patch is a
> page. Each revision of the patch is uploaded to the page that is
> assigned to the patch.
>
>
>> 2. When a patch is outright rejected, the patch author updates the
>> status accordingly.
>
> I don't find this realistic. I believe we will just end up trolling
> back through patch archives trying to remember what the status was.

The idea is that a commit fest lasts for a short period, ideally a week
or so. If the patch author drops the ball at this point, and doesn't
respond to requests to close the case, it should be bright in everyone's
mind that a patch has been rejected, and someone else can then go and
mark it as such.

>> 3. Many patches will not be ready for committing yet, because there's
>> bugs that need fixing, or it needs performance testing or whatever.
>> If it's a quick thing, patch author can just submit an updated patch,
>> or test results or whatever and continue discussion. Otherwise, after
>> author knows what he's going to do next, he updates the status on the
>> wiki accordingly. The status can be something like "will do
>> performance testing", "will try approach suggested by X", "will clean
>> up comments" etc.
>
> I assume this happens "After" discussion on -hackers?

The discussion and review will happen on -hackers / patches. After the
author thinks he knows what he needs to do next, he will update the status.

If he doesn't update the status, he will start to get mails like "where
are you on this?" and "what's up dude, didn't you understand what you
were told?" fairly soon.

>> The trick here is that the patch authors drive the process. An author
>
> Yes and I believe it is a trick that is destined to bomb at the magic
> show. We can't convince hackers to follow standard bug tracker policies
> but we are going to convince them to keep a mailing list + wiki up to
> date?

Mailing list would function as they do now. The commitfests are there to
help patch authors to get attention to their work. If you don't want to
participate, or you can get that attention through other means, like
just sending a patch to -patches and cross your fingers, that's
perfectly fine.

I think we'll have more success convincing patch authors to update a
wiki page, than we'll have to convince reviewers to do so. I know that's
true at least for me. If I want people to review my patch, I'm ready to
sing and dance if that's what it takes. But if there's extra steps in
reviewing a patch, I might just not bother.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Fri, 07 Mar 2008 18:46:24 +0000
"Heikki Linnakangas" <heikki(at)enterprisedb(dot)com> wrote:

> I think we'll have more success convincing patch authors to update a
> wiki page, than we'll have to convince reviewers to do so. I know
> that's true at least for me. If I want people to review my patch, I'm
> ready to sing and dance if that's what it takes. But if there's extra
> steps in reviewing a patch, I might just not bother.

Well that is what my email is about, dropping extra steps :).

Joshua D. Drake

- --
The PostgreSQL Company since 1997: http://www.commandprompt.com/
PostgreSQL Community Conference: http://www.postgresqlconference.org/
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
PostgreSQL SPI Liaison | SPI Director | PostgreSQL political pundit

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFH0Y6JATb/zqfZUUQRAsgmAKCGXETMi2lwPZpnSCXnULRNOcRYpACfQLU3
aTR+vO0a8RoXpSvoTJE1RpI=
=ABUV
-----END PGP SIGNATURE-----

Joshua D. Drake wrote:

> On Fri, 07 Mar 2008 18:46:24 +0000
> "Heikki Linnakangas" <heikki(at)enterprisedb(dot)com> wrote:
>
> > I think we'll have more success convincing patch authors to update a
> > wiki page, than we'll have to convince reviewers to do so. I know
> > that's true at least for me. If I want people to review my patch, I'm
> > ready to sing and dance if that's what it takes. But if there's extra
> > steps in reviewing a patch, I might just not bother.
>
> Well that is what my email is about, dropping extra steps :).

I agree that that's a good objective, but I think a Wiki makes for a
crappy patch tracker.

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Alvaro Herrera wrote:
> Joshua D. Drake wrote:
>
>> On Fri, 07 Mar 2008 18:46:24 +0000
>> "Heikki Linnakangas" <heikki(at)enterprisedb(dot)com> wrote:
>>
>>> I think we'll have more success convincing patch authors to update a
>>> wiki page, than we'll have to convince reviewers to do so. I know
>>> that's true at least for me. If I want people to review my patch, I'm
>>> ready to sing and dance if that's what it takes. But if there's extra
>>> steps in reviewing a patch, I might just not bother.
>> Well that is what my email is about, dropping extra steps :).
>
> I agree that that's a good objective, but I think a Wiki makes for a
> crappy patch tracker.

Sure, but let's not turn this into a bug/patch tracker discussion,
please :-/. A wiki is not ideal, but it's there.

The main point of my proposal is: let's make the *authors* who want
their stuff to be reviewed as part of a commitfest do the extra work.
There would be no extra work required for patch reviewers.

Sure, we can refine that later. making it easier for patch authors as
well, but I don't think it's an unreasonable amount of work to keep one
line per patch up-to-date in a wiki. The line doesn't need to contain
anything else than title of patch, name of the author, and links to
latest patch and relevant discussion threads, if applicable. And
commitfests are short, you only need to update the wiki a couple of
times during the commitfest.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

> Heikki Linnakangas wrote:
> The main point of my proposal is: let's make the *authors* who want
> their stuff to be reviewed as part of a commitfest do the extra work.
> There would be no extra work required for patch reviewers.
>

I think this makes the most sense. It distributes the work to authors
who know the most about the patch/feature and have probably followed all
discussions related to it. Updating a wiki or something similar is a
brainless activity.

--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/

Heikki Linnakangas wrote:
> Alvaro Herrera wrote:
>> Joshua D. Drake wrote:
>>
>>> On Fri, 07 Mar 2008 18:46:24 +0000
>>> "Heikki Linnakangas" <heikki(at)enterprisedb(dot)com> wrote:
>>>
>>>> I think we'll have more success convincing patch authors to update a
>>>> wiki page, than we'll have to convince reviewers to do so. I know
>>>> that's true at least for me. If I want people to review my patch, I'm
>>>> ready to sing and dance if that's what it takes. But if there's extra
>>>> steps in reviewing a patch, I might just not bother.
>>> Well that is what my email is about, dropping extra steps :).
>>
>> I agree that that's a good objective, but I think a Wiki makes for a
>> crappy patch tracker.
>
> Sure, but let's not turn this into a bug/patch tracker discussion,
> please :-/. A wiki is not ideal, but it's there.

Yeah, let's keep focus on *this* commitfest for now. The only chance
anything will be used for that is if it exists, in production, for
postgresql, *today*.

If we want something else in the future, sure. Let's do this as an
iterative process.

> The main point of my proposal is: let's make the *authors* who want
> their stuff to be reviewed as part of a commitfest do the extra work.
> There would be no extra work required for patch reviewers.

I think that's perfectly reasonable. There *will* be small patches that
fall through the cracks of that, but we can deal with those outside the
process for now, I think.

//Magnus

Heikki Linnakangas wrote:
>
> The main point of my proposal is: let's make the *authors* who want
> their stuff to be reviewed as part of a commitfest do the extra work.
> There would be no extra work required for patch reviewers.
I agree with Heikki that for the process to be successful, it should
not impose extra work for the reviewers. The author is more motivated to
get his/her patch in than the review to review the patch. Besides, I
don't think it's too much to ask to have the author enter one line into
a wiki and keep the status up to date for the duration of the commitfest.
>
> Sure, we can refine that later. making it easier for patch authors as
> well, but I don't think it's an unreasonable amount of work to keep
> one line per patch up-to-date in a wiki. The line doesn't need to
> contain anything else than title of patch, name of the author, and
> links to latest patch and relevant discussion threads, if applicable.
> And commitfests are short, you only need to update the wiki a couple
> of times during the commitfest.
>
I think it's a good idea to try out the process with a wiki first, and
if it works well, consider a more sophisticated tool if needed.

How about use the following fields for the wiki page and have the author
be responsible for keeping it up to date?

Patch title
Patch URL
Discussion URL (optional)
Author
Reviewer (updated by reviewer or author)
Patch Status (awaiting review -> reviewing -> ... -> accepted |
rejected | whatever)

Regards,
-Robert

Andrew Chernow wrote:
>
>> Heikki Linnakangas wrote:
>> The main point of my proposal is: let's make the *authors* who want
>> their stuff to be reviewed as part of a commitfest do the extra work.
>> There would be no extra work required for patch reviewers.
>>
>
> I think this makes the most sense. It distributes the work to authors
> who know the most about the patch/feature and have probably followed
> all discussions related to it. Updating a wiki or something similar
> is a brainless activity.

We are making much too big a deal of not very much here. AIUI a
commitfest just involves a change in priority of tasks, mainly by
committers, but also to some extent by other developers capable of doing
reviews.

As for making authors do extra work: good luck with that.

In any case - ideally there shouldn't be any extra work.

If someone wants to turn the patch list into something more
reader-friendly, then that's a separate issue, really, quite apart from
the commitfest. I rather liked the wiki page that Stefan Kaltenbrunner
maintained for 8.4 features - it provided quite a good overview of where
we were.

cheers

andrew

On Fri, 7 Mar 2008, Heikki Linnakangas wrote:

> If I want people to review my patch, I'm ready to sing and dance if
> that's what it takes.

Great timing, there's even a suitable song available for you today:
http://use.perl.org/~grantm/journal/35855

--
* Greg Smith gsmith(at)gregsmith(dot)com http://www.gregsmith.com Baltimore, MD

"Heikki Linnakangas" <heikki(at)enterprisedb(dot)com> writes:
> Sure, we can refine that later. making it easier for patch authors as
> well, but I don't think it's an unreasonable amount of work to keep one
> line per patch up-to-date in a wiki. The line doesn't need to contain
> anything else than title of patch, name of the author, and links to
> latest patch and relevant discussion threads, if applicable. And
> commitfests are short, you only need to update the wiki a couple of
> times during the commitfest.

This is reasonable for the sort of medium-to-large patch that the author
has put a lot of time into. But we also get a lot of small one-off
patches where it's not so reasonable. Now of course many of those get
applied right away, but not all. One of the services that Bruce's patch
queue has always performed is making sure stuff like that doesn't fall
through the cracks. I don't think a purely author-driven patch queue
will work to ensure that.

We could combine the ideas: encourage authors to use the wiki, but have
someone (probably Bruce ;-)) in charge of adding stuff to the wiki if
the author doesn't.

regards, tom lane

Tom Lane wrote:
> This is reasonable for the sort of medium-to-large patch that the author
> has put a lot of time into. But we also get a lot of small one-off
> patches where it's not so reasonable. Now of course many of those get
> applied right away, but not all. One of the services that Bruce's patch
> queue has always performed is making sure stuff like that doesn't fall
> through the cracks. I don't think a purely author-driven patch queue
> will work to ensure that.
>
> We could combine the ideas: encourage authors to use the wiki, but have
> someone (probably Bruce ;-)) in charge of adding stuff to the wiki if
> the author doesn't.
>
Stefan Kaltenbrunner said a script can be written to insert an entry
into the wiki automatically when a new patch comes in. That would be
ideal and would avoid manual intervention. The author can still update
the wiki as needed.

http://archives.postgresql.org/pgsql-hackers/2008-02/msg00433.php

Regards,
-Robert

> This is reasonable for the sort of medium-to-large patch that the author
> has put a lot of time into. But we also get a lot of small one-off
> patches where it's not so reasonable. Now of course many of those get
> applied right away, but not all.
>

Just a thought... maybe a distinction should be made between "quick-fix" patches
and things that would require more review due to behavior changes or new
features. Seems reasonable to treat something like HOT and a one line patch
differently.

Not sure where the magic line would be drawn: size of patch, is it a bug fix or
a new feature, requires extensive testing (more time), etc... I do think it is
overkill to have a wiki entry for a one line patch with a 30 minute life-span
(either thrown out or applied).

--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/

On 08/03/2008, Heikki Linnakangas <heikki(at)enterprisedb(dot)com> wrote:
> I think we'll have more success convincing patch authors to update a
> wiki page, than we'll have to convince reviewers to do so. I know that's
> true at least for me. If I want people to review my patch, I'm ready to
> sing and dance if that's what it takes. But if there's extra steps in
> reviewing a patch, I might just not bother.

+1. As a patch author, I have much more personal investment in a
patch than anyone else, and I'm happy to maintain a wiki page if it's
going to get my patches through the process more efficiently.

But I also agree with Josh Drake's comment about a single point of
entry. If patch authors are updating the wiki, and reviewers are
using the wiki to guide their efforts, what purpose does the -patches
mailing list serve? Does sending an email to -patches on top of
submitting the patch on the wiki actually buy us anything? It seems
redundant.

Regards,
BJ

Brendan Jurd wrote:
> But I also agree with Josh Drake's comment about a single point of
> entry. If patch authors are updating the wiki, and reviewers are
> using the wiki to guide their efforts, what purpose does the -patches
> mailing list serve? Does sending an email to -patches on top of
> submitting the patch on the wiki actually buy us anything? It seems
> redundant.

The mail to -patches will contain a detailed description of the patch,
the wiki item is just a one-liner. All discussions will happen on the
mailing list. If there's no initial mail there, there's nothing to click
"reply" on. And mailing lists provide an archive.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com