Thread: 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@enterprisedb.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@enterprisedb.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@enterprisedb.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@enterprisedb.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@enterprisedb.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@enterprisedb.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@gregsmith.com http://www.gregsmith.com Baltimore, MD
"Heikki Linnakangas" <heikki@enterprisedb.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@enterprisedb.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