Thread: Reviewfest 2010-06 Plans and Call for Reviewers
Folks, The PostgreSQL 9.1 Development Plan: http://wiki.postgresql.org/wiki/PostgreSQL_9.1_Development_Plan calls for a ReviewFest to run from the 15th of June (tomorrow) until the start of the first CommitFest for the 9.1 release. The idea is that those with time available to contribute beyond what they can usefully contribute to getting 9.0 released can help provide feedback on patches submitted so far, to lighten the load of the CF proper when it starts. I have agreed to manage this RF. Of course, we also need reviewers. I do want to emphasize that we *don't* want this process to impact the release of 9.0; it is in the best interest of everyone that 9.0 is tested, stable, and released as soon as practicable. Please think hard about whether there is some testing or review you could do to facilitate the 9.0 release effort, and only participate in this RF to the extent that it doesn't detract from the other effort. Also, in testing these patches, be alert to any problems in the *before* version -- you may find 9.0 issues in the process of attempting to test these patches, and such issues, if found, should take priority. If you find a possible 9.0 issue, please set aside efforts to review the patch until you have pursued the preexisting issue. This is essentially being treated as an early start on the 2010-07 CF, so that is where the process will be managed: https://commitfest.postgresql.org/action/commitfest_view?id=6 Note that we don't expect any commits for these patches to happen until after the 9.0 stable branch is created and committers are done with their 9.0 release efforts, most likely some time after the 2010-07 CF is officially in progress. Also, we probably won't be bumping many patches to "returned with feedback" status during the RF; the apparent work required would need to be more than could reasonably be expected to be completed for the CF. Before signing up, please review these pages, to get an idea what's involved: http://wiki.postgresql.org/wiki/Reviewing_a_Patch http://wiki.postgresql.org/wiki/RRReviewers On the lighter side: http://wiki.postgresql.org/images/5/58/11_eggyknap-patch-review.pdf Please send me an email (without copying the list) if you are available to review; feel free to include any information that might be helpful in assigning you an appropriate patch. -Kevin
Hi! On Mon, Jun 14, 2010 at 9:25 AM, Kevin Grittner <Kevin.Grittner@wicourts.gov> wrote: > http://wiki.postgresql.org/wiki/PostgreSQL_9.1_Development_Plan > > calls for a ReviewFest to run from the 15th of June (tomorrow) until > the start of the first CommitFest for the 9.1 release. The idea is > that those with time available to contribute beyond what they can > usefully contribute to getting 9.0 released can help provide > feedback on patches submitted so far, to lighten the load of the CF > proper when it starts. I have agreed to manage this RF. I'm helping out too :) A few folks from PDXPUG are meeting up this evening to go over some patches. We'll be on IRC at #pdxpug from 6pm-8pm PDT if you want to join in virtually. We'll post our results tomorrow! -selena -- http://chesnok.com/daily - me
> A few folks from PDXPUG are meeting up this evening to go over some
> patches. We'll be on IRC at #pdxpug from 6pm-8pm PDT if you want to
> join in virtually.
We'll try to organize an SFPUG review for next month, shortly before the
commitfest starts.
--
-- Josh Berkus
PostgreSQL Experts Inc.
http://www.pgexperts.com
On Mon, Jun 14, 2010 at 12:25 PM, Kevin Grittner <Kevin.Grittner@wicourts.gov> wrote: > The PostgreSQL 9.1 Development Plan: > > http://wiki.postgresql.org/wiki/PostgreSQL_9.1_Development_Plan > > calls for a ReviewFest to run from the 15th of June (tomorrow) until > the start of the first CommitFest for the 9.1 release. The idea is > that those with time available to contribute beyond what they can > usefully contribute to getting 9.0 released can help provide > feedback on patches submitted so far, to lighten the load of the CF > proper when it starts. I have agreed to manage this RF. Who is going to manage the actual CF? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company
Robert Haas <robertmhaas@gmail.com> wrote: > Who is going to manage the actual CF? I'm willing to do so if people want that. I promise to be more aggressive about pursuing this once the release is out; I've seen some of the same people who are doing reviews contributing to the release process, so I haven't wanted to push on the RF tasks. I'm also perfectly willing to step aside for anyone else. ;-) -Kevin
On Thu, Jul 8, 2010 at 9:32 AM, Kevin Grittner <Kevin.Grittner@wicourts.gov> wrote: > Robert Haas <robertmhaas@gmail.com> wrote: > >> Who is going to manage the actual CF? > > I'm willing to do so if people want that. I promise to be more > aggressive about pursuing this once the release is out; I've seen > some of the same people who are doing reviews contributing to the > release process, so I haven't wanted to push on the RF tasks. > > I'm also perfectly willing to step aside for anyone else. ;-) Personally, if you can step up to the plate on this one, I think that would be great. I am going to be out of town for a week starting two days from now, and I would rather not have to try to manage a CF without regular Internet connectivity and while I'm supposed to be on vacation. And, if I do spend some time working on this, either while I'm on vacation or afterwards, it would be nice if I could focus on reviewing and committing. If someone else wants to volunteer, of course, that's great too... In terms of being aggressive about pursuing this, I think it's important that between July 15th and August 14th we try to give everyone who has submitted a patch by July 14th some feedback on their work, which means we need more people reviewing than have so far. I don't think the release will be out before the CF is over, but I'm hoping that we can get enough people involved to walk and chew gum at the same time. Last year, the first CommitFest involved returning with feedback a lot of patches that had horribly bitrotted or had major design issues - and the second CommitFest involved committing many of those same patches. So giving people feedback now is really important to avoid having things pile up at the end of the release cycle. The good/bad news is that we don't have any major uncommitted patches floating around unmerged at this pont, as we did last cycle with HS and SR. KNNGIST is in a similar situation to where HS and SR were at this time last year, but it's not as big of a patch, and it's not as high-profile (although I think anyone who uses PostGIS, and not a few others, are drooling...). Sync rep is going to be a big patch, or, hopefully, a series of patches, but it's more of a question of getting it designed and written than getting it merged at this point. I have a bad feeling we're going to be trying to land that one at the very last minute. :-( -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company
Robert Haas <robertmhaas@gmail.com> wrote: > Last year, the first CommitFest involved returning with feedback > a lot of patches that had horribly bitrotted That's an excellent point. If there is anyone out there who could do an initial run to kick back patches with bitrot in the next week or so, that would be a tremendous help. > or had major design issues That's a much harder problem. The set of people who can review for that is rather smaller than the set who can see if a patch applies without error. > So giving people feedback now is really important to avoid having > things pile up at the end of the release cycle. Yeah, with only four review cycles, big items must get some review early to avoid holding up the release at the other end. > The good/bad news is that we don't have any major uncommitted > patches floating around unmerged at this pont, as we did last > cycle with HS and SR. I'm not sure I understand what you mean by "unmerged" -- are you excluding patches where they are in a git repository which is merging in HEAD on a regular basis? -Kevin
On Thu, Jul 8, 2010 at 11:36 AM, Kevin Grittner <Kevin.Grittner@wicourts.gov> wrote: >> or had major design issues > > That's a much harder problem. The set of people who can review for > that is rather smaller than the set who can see if a patch applies > without error. Well, true. But reporting whether the patch applies without error is about the most minimal review possible, so that's not saying much. Many of the patches that need review are relatively small ones, so we need people to check whether they work, see if they have docs and regression tests, try to break them, and also - importantly - offer an opinion on whether the proposed feature is actually useful. Of course others may disagree, but you can't reach consensus if nobody's willing to offer a starting point for discussion. Reviewers who can actually analyze the code even a little bit can find many more issues. Does it fit the style of the surrounding code? Are there unnecessary or unrelated changes in the patch? And for extra credit, does it have bugs? What I do frequently is find a "comparable" - something similar in the existing code - and compare them side-by-side. For each change, I ask myself whether it's there because the new functionality is different from the old, or whether it's arbitrary. I agree that the larger features need a different kind of analysis, and if those get left for more experienced reviewers I think that's fine. Hopefully, some of those people will volunteer to help out; I noticed that Itagaki Takahiro has already started reviewing, and Bernd Helmle has signed up to review one of my patches. But there is no shortage of things that can be looked at by people who are doing this first the first time, especially if they have even a passing ability to read C code. >> The good/bad news is that we don't have any major uncommitted >> patches floating around unmerged at this pont, as we did last >> cycle with HS and SR. > > I'm not sure I understand what you mean by "unmerged" -- are you > excluding patches where they are in a git repository which is > merging in HEAD on a regular basis? By unmerged I meant simply uncommitted. I know we have a number of fairly big patches, but I don't think they're as complex as HS and SR, and they are not leftovers that were too big to swallow during the 9.0 cycle. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company
Robert Haas <robertmhaas@gmail.com> wrote:
> Kevin Grittner <Kevin.Grittner@wicourts.gov> wrote:
>>> or had major design issues
>>
>> That's a much harder problem. The set of people who can review
>> for that is rather smaller than the set who can see if a patch
>> applies without error.
>
> Well, true. But reporting whether the patch applies without error
> is about the most minimal review possible
I didn't mean to imply that only the two extremes of review ("does
the patch apply?" and "does this patch have a major overall design
flaw?") were the only things to try to address now; I was just
responding to your observation that these comprised a lot of the
activity of the first CommitFest last year, and that the latter is
harder to address without a review by a senior developer. I suspect
that one person could check for bitrot in all pending patches with a
one or two FTE day's effort, and if that's done within the next few
days, it might allow time for fixes before the start of the CF free
up more of the first week of the CF to more substantive review.
Nice comments later in the email, though; I hope you won't mind if
you find excerpts popping up in the code review Wiki pages. ;-)
-Kevin
On Thu, Jul 8, 2010 at 12:44 PM, Kevin Grittner <Kevin.Grittner@wicourts.gov> wrote: > Nice comments later in the email, though; I hope you won't mind if > you find excerpts popping up in the code review Wiki pages. ;-) Not at all. Go for it... -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company