Thread: No answers on CommitFest procedures?
Folks, Have received exactly zero feedback on the question of whether I should be assigning reviewers to "WIP" patches or not. --Josh
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
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
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
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.
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
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.
"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!