Thread: Commitfest II CLosed
Thanks very much to Mike Blackwell and Craig Kerstiens for their persistence through what most people would consider a tedious and thankless task. Thanks also to the patch submitters, reviewers and other participants. That the formal commitfest is over does not mean that your patch won't get reviewed or committed until November. What it does mean is that people will be setting patch review as a lower priority, frequently so they can live their lives, work on new stuff, do their day jobs... We got 20 patches, many quite significant, committed this time. Kudos! Cheers, David. -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
Hi, 2013-10-19 17:20 keltezéssel, David Fetter írta: > Thanks very much to Mike Blackwell and Craig Kerstiens for their > persistence through what most people would consider a tedious and > thankless task. Thanks also to the patch submitters, reviewers and > other participants. > > That the formal commitfest is over does not mean that your patch won't > get reviewed or committed until November. What it does mean is that > people will be setting patch review as a lower priority, frequently so > they can live their lives, work on new stuff, do their day jobs... > > We got 20 patches, many quite significant, committed this time. > Kudos! > > Cheers, > David. what will happen to patches left in pending state in the 2013-09 CF? Best regards, Zoltán Böszörményi -- ---------------------------------- Zoltán Böszörményi Cybertec Schönig & Schönig GmbH Gröhrmühlgasse 26 A-2700 Wiener Neustadt, Austria Web: http://www.postgresql-support.de http://www.postgresql.at/
On Sun, Oct 20, 2013 at 10:42:10AM +0200, Boszormenyi Zoltan wrote: > Hi, > > 2013-10-19 17:20 keltezéssel, David Fetter írta: > >Thanks very much to Mike Blackwell and Craig Kerstiens for their > >persistence through what most people would consider a tedious and > >thankless task. Thanks also to the patch submitters, reviewers and > >other participants. > > > >That the formal commitfest is over does not mean that your patch won't > >get reviewed or committed until November. What it does mean is that > >people will be setting patch review as a lower priority, frequently so > >they can live their lives, work on new stuff, do their day jobs... > > > >We got 20 patches, many quite significant, committed this time. > >Kudos! > > > >Cheers, > >David. > > what will happen to patches left in pending state in the 2013-09 CF? I have moved them to the next CF. This does not mean that they are abandoned until then. I strongly suspect that people will be reviewing and committing many of them between now and then. Cheers, David. -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
On 2013-10-20 08:12:37 -0700, David Fetter wrote: > > what will happen to patches left in pending state in the 2013-09 CF? > > I have moved them to the next CF. This does not mean that they are > abandoned until then. I strongly suspect that people will be > reviewing and committing many of them between now and then. -1 for doing this in the future. The point of the CF is exactly that all patches get at least one good round of review. Moving unreviewed patches to the next CF will let them just suffer the same fate there. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On 10/21/13 1:31 AM, Andres Freund wrote: > The point of the CF is exactly that all > patches get at least one good round of review. Moving unreviewed patches > to the next CF will let them just suffer the same fate there. What is the alternative?
On 2013-10-21 09:15:36 -0400, Peter Eisentraut wrote: > On 10/21/13 1:31 AM, Andres Freund wrote: > > The point of the CF is exactly that all > > patches get at least one good round of review. Moving unreviewed patches > > to the next CF will let them just suffer the same fate there. > > What is the alternative? I am not 100% sure, but what's the point of the CF if we're not actually reviewing patches that wouldn't get review without it? So I guess it's not starting the next one before we've finished - which we obviously haven't in this case - the last one. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On 21.10.2013 16:15, Peter Eisentraut wrote: > On 10/21/13 1:31 AM, Andres Freund wrote: >> The point of the CF is exactly that all >> patches get at least one good round of review. Moving unreviewed patches >> to the next CF will let them just suffer the same fate there. Agreed. People have different views on what the purpose of a commitfest is, but IMO the point is to make sure that every patch submitted gets at least a cursory review in a timely fashion. Pushing patches to the next one because no-one has gotten around to review them is a failure. > What is the alternative? If no-one really cares enough about a patch to review it, mark it as "rejected, because no-one but the patch author cares". Harsh, but that's effectively what pushing to the next commitfest means anyway. Better to be honest about it. At least that way the author can promote the patch's virtues more on the mailing list, or personally contact someone who might be interested, to get some attention, and resubmit if he thinks that it might have a chance on the next commitfest. Another alternative is to push harder to make sure that every patch gets some review. I don't know how to accomplish that. Robert Haas did a great job at that in the first few commitfests (IIRC), but only because he personally spent a lot of time not only managing the commitfest but actually reviewing the patches that no-one else bothered with. That's a great way to make sure that every patch gets some attention, but I don't think we have any takers for that role. I feel guilty to complain, while not actually volunteering to be a commitfest manager myself, but I wish the commitfest manager would be more aggressive in nagging, pinging and threatening people to review stuff. If nothing else, always feel free to nag me :-). Josh tried that with the infamous Slacker List, but that backfired. Rather than posting a public list of shame, I think it would work better to send short off-list nag emails, or chat via IM. Something like "Hey, you've signed up to review this. Any progress?". Or "Hey, could you take a look at X please? No-one else seems to care about it." - Heikki
On 10/21/2013 03:56 PM, Heikki Linnakangas wrote: > > I feel guilty to complain, while not actually volunteering to be a > commitfest manager myself, but I wish the commitfest manager would be > more aggressive in nagging, pinging and threatening people to review > stuff. If nothing else, always feel free to nag me :-). Josh tried > that with the infamous Slacker List, but that backfired. Rather than > posting a public list of shame, I think it would work better to send > short off-list nag emails, or chat via IM. Something like "Hey, you've > signed up to review this. Any progress?". Or "Hey, could you take a > look at X please? No-one else seems to care about it." Or maybe even nag publicly with "list of orphans" - hey people, do you *really* think that this patch is not needed ? -- Hannu Krosing PostgreSQL Consultant Performance, Scalability and High Availability 2ndQuadrant Nordic OÜ
Heikki Linnakangas <hlinnakangas@vmware.com> writes:
> On 21.10.2013 16:15, Peter Eisentraut wrote:
>> What is the alternative?
> If no-one really cares enough about a patch to review it, mark it as
> "rejected, because no-one but the patch author cares". Harsh, but that's
> effectively what pushing to the next commitfest means anyway.
Well, that could be the problem, but it's also possible that no one could
get to it in the alloted CF timeframe. Maybe the best-qualified reviewers
were on vacation, or maybe there were just too many patches. I could see
bouncing a patch on this basis if it doesn't get touched for, say, two
consecutive CFs.
regards, tom lane
On Mon, Oct 21, 2013 at 9:18 AM, Andres Freund <andres@2ndquadrant.com> wrote: > On 2013-10-21 09:15:36 -0400, Peter Eisentraut wrote: >> On 10/21/13 1:31 AM, Andres Freund wrote: >> > The point of the CF is exactly that all >> > patches get at least one good round of review. Moving unreviewed patches >> > to the next CF will let them just suffer the same fate there. >> >> What is the alternative? > > I am not 100% sure, but what's the point of the CF if we're not actually > reviewing patches that wouldn't get review without it? So I guess it's > not starting the next one before we've finished - which we obviously > haven't in this case - the last one. Yeah. There were a huge number of patches in this CommitFest that sat around in the waiting on author state for hugely long periods of time.One of the critical functions of the CommitFest manager(s)IMV is to make sure that patches that are in that state get pushed to Returned with Feedback so that it's more obvious which things are still alive and kicking. That really wasn't done until about a week before the end of the CommitFest, when I stepped in and did some of it. But that really needs to be more of an ongoing process. Supposedly, we have a policy that for each patch you submit, you ought to review a patch. That right there ought to provide enough reviewers for all the patches, but clearly it didn't. And I'm pretty sure that some people (like me) looked at a lot MORE patches than they themselves submitted. I think auditing who is not contributing in that area and finding tactful ways to encourage them to contribute would be a very useful service to the project. Finally, I think we need to have some discussion of the patches that are ready for committer but got punted, and see if we can figure out whether any committer has plans to look at them. Those patches are: Extension Templates - I think Peter Eisentraut commented on this one at some stage, but I'm not sure if he's planning to work further on it. UNNEST with multiple args, and TABLE with multiple functions - Heikki did some work on this, maybe he's planning to commit it? Numeric Aggregates Performance Improvement - I looked at this one previously so should probably look it over again. Statistics collection for CLUSTER command - Noah recommended rejecting this on performance grounds. Maybe we should do that. simple date time constructors - Alvaro previously looked at this, but I don't know whether he plans to work on it further. simple LO API - no committer interest to my knowledge Bugfix for timeout in LDAP connection parameter resolution - I think Peter Eisentraut is planning to commit this -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Actually, I did call them out in the thread announcing the CF Wrap Up (http://www.postgresql.org/message-id/CAESHdJonURj3i9HR2w4e=ohep5Hx7sNqyyDSGYWeQqa+A3dfEg@mail.gmail.com).
Looking back, it may have been better to post it as a separate thread, but I'm not confident that would have made much difference.
__________________________________________________________________________________
Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley
1750 Wallace Ave | St Charles, IL 60174-3401
Office: 630.313.7818
Mike.Blackwell@rrd.com
http://www.rrdonnelley.com
On Mon, Oct 21, 2013 at 9:45 AM, Hannu Krosing <hannu@2ndquadrant.com> wrote:
On 10/21/2013 03:56 PM, Heikki Linnakangas wrote:Or maybe even nag publicly with "list of orphans" - hey people, do you
>
> I feel guilty to complain, while not actually volunteering to be a
> commitfest manager myself, but I wish the commitfest manager would be
> more aggressive in nagging, pinging and threatening people to review
> stuff. If nothing else, always feel free to nag me :-). Josh tried
> that with the infamous Slacker List, but that backfired. Rather than
> posting a public list of shame, I think it would work better to send
> short off-list nag emails, or chat via IM. Something like "Hey, you've
> signed up to review this. Any progress?". Or "Hey, could you take a
> look at X please? No-one else seems to care about it."
*really* think that this patch is not needed ?
--
Hannu Krosing
PostgreSQL Consultant
Performance, Scalability and High Availability
2ndQuadrant Nordic OÜ
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 10/21/2013 05:13 PM, Mike Blackwell wrote:
I was more thinking in lines of creating one thread per unreviewedActually, I did call them out in the thread announcing the CF Wrap Up (http://www.postgresql.org/message-id/CAESHdJonURj3i9HR2w4e=ohep5Hx7sNqyyDSGYWeQqa+A3dfEg@mail.gmail.com).Looking back, it may have been better to post it as a separate thread, but I'm not confident that would have made much difference.
patch with patch title in Subject:, so that each could become its
own discussion
-- Hannu Krosing PostgreSQL Consultant Performance, Scalability and High Availability 2ndQuadrant Nordic OÜ
__________________________________________________________________________________
Mike Blackwell | Technical Analyst, Distribution Services/Rollout Management | RR Donnelley
1750 Wallace Ave | St Charles, IL 60174-3401
Office: 630.313.7818
Mike.Blackwell@rrd.com
http://www.rrdonnelley.comOn Mon, Oct 21, 2013 at 9:45 AM, Hannu Krosing <hannu@2ndquadrant.com> wrote:On 10/21/2013 03:56 PM, Heikki Linnakangas wrote:Or maybe even nag publicly with "list of orphans" - hey people, do you
>
> I feel guilty to complain, while not actually volunteering to be a
> commitfest manager myself, but I wish the commitfest manager would be
> more aggressive in nagging, pinging and threatening people to review
> stuff. If nothing else, always feel free to nag me :-). Josh tried
> that with the infamous Slacker List, but that backfired. Rather than
> posting a public list of shame, I think it would work better to send
> short off-list nag emails, or chat via IM. Something like "Hey, you've
> signed up to review this. Any progress?". Or "Hey, could you take a
> look at X please? No-one else seems to care about it."
*really* think that this patch is not needed ?
--
Hannu Krosing
PostgreSQL Consultant
Performance, Scalability and High Availability
2ndQuadrant Nordic OÜ
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
2013-10-21 17:11 keltezéssel, Robert Haas írta: > On Mon, Oct 21, 2013 at 9:18 AM, Andres Freund <andres@2ndquadrant.com> wrote: >> On 2013-10-21 09:15:36 -0400, Peter Eisentraut wrote: >>> On 10/21/13 1:31 AM, Andres Freund wrote: >>>> The point of the CF is exactly that all >>>> patches get at least one good round of review. Moving unreviewed patches >>>> to the next CF will let them just suffer the same fate there. >>> What is the alternative? >> I am not 100% sure, but what's the point of the CF if we're not actually >> reviewing patches that wouldn't get review without it? So I guess it's >> not starting the next one before we've finished - which we obviously >> haven't in this case - the last one. > Yeah. There were a huge number of patches in this CommitFest that sat > around in the waiting on author state for hugely long periods of time. > One of the critical functions of the CommitFest manager(s) IMV is to > make sure that patches that are in that state get pushed to Returned > with Feedback so that it's more obvious which things are still alive > and kicking. That really wasn't done until about a week before the > end of the CommitFest, when I stepped in and did some of it. But that > really needs to be more of an ongoing process. > > Supposedly, we have a policy that for each patch you submit, you ought > to review a patch. That right there ought to provide enough reviewers > for all the patches, but clearly it didn't. And I'm pretty sure that > some people (like me) looked at a lot MORE patches than they > themselves submitted. I think auditing who is not contributing in > that area and finding tactful ways to encourage them to contribute > would be a very useful service to the project. I wanted to get to this point, too. I hoped that reviewing 4 patches in this CF (UNNEST, Extension templates, DISCARD SEQUENCES, and extended RETURNING syntax) gets my huge patch reviewed. I even provided a repo @github where it was broken up into pieces that can be sanely reviewed. It still wasn't enough. Even Michael Meskes (ECPG is his pet project) and the guy @Fujitsu who contacted me privately and expressed interest in this patch didn't chime in. As a social experiment, the CF looks like a clear failure from this seat of mine. (Sorry.) > > Finally, I think we need to have some discussion of the patches that > are ready for committer but got punted, and see if we can figure out > whether any committer has plans to look at them. Those patches are: > > Extension Templates - I think Peter Eisentraut commented on this one > at some stage, but I'm not sure if he's planning to work further on > it. > UNNEST with multiple args, and TABLE with multiple functions - Heikki > did some work on this, maybe he's planning to commit it? > Numeric Aggregates Performance Improvement - I looked at this one > previously so should probably look it over again. > Statistics collection for CLUSTER command - Noah recommended rejecting > this on performance grounds. Maybe we should do that. > simple date time constructors - Alvaro previously looked at this, but > I don't know whether he plans to work on it further. > simple LO API - no committer interest to my knowledge > Bugfix for timeout in LDAP connection parameter resolution - I think > Peter Eisentraut is planning to commit this > -- ---------------------------------- Zoltán Böszörményi Cybertec Schönig & Schönig GmbH Gröhrmühlgasse 26 A-2700 Wiener Neustadt, Austria Web: http://www.postgresql-support.de http://www.postgresql.at/
Zoltan,
* Boszormenyi Zoltan (zb@cybertec.at) wrote:
> I even provided a repo @github where it was broken up into pieces that can be sanely reviewed.
You also gave the first person looking at the patch a hard time about
asking for it to be broken up; unnecessairly, imv. Thanks for breaking
it up and for doing patch review of other patches- it does help the
project move forward. Try to be understanding when someone asks a
question that's already been answered; we're all quite busy and may
forget or miss things.
I don't know if Alvaro will have time to look into this or if he perhaps
already has, but I had noticed this patch earlier and it was one of the
ones I had hoped to take a look at, as I've been in the ECPG bits due to
the work with Coverity that I've been doing. I'll try and provide
feedback later this week/weekend on it.
Thanks,
Stephen
Tom, >> If no-one really cares enough about a patch to review it, mark it >> as "rejected, because no-one but the patch author cares". Harsh, >> but that's effectively what pushing to the next commitfest means >> anyway. > > Well, that could be the problem, but it's also possible that no one > could get to it in the alloted CF timeframe. Maybe the > best-qualified reviewers were on vacation, or maybe there were just > too many patches. I could see bouncing a patch on this basis if it > doesn't get touched for, say, two consecutive CFs. That would be more or less a declaration of failure by this project to regulate our own development process, and an abandonment of the idea of ever getting new contributors. If we don't guarantee legit patches at least one review, why would anyone submit code to this project at all? At some point folks on this list are going to admit that we have a serious problem with reviews and reviewers, and that it's worth a project-wide effort to do something about it. Apparently that day hasn't come yet; most people are still in denial. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
Boszormenyi Zoltan escribió: > I hoped that reviewing 4 patches in this CF (UNNEST, Extension templates, > DISCARD SEQUENCES, and extended RETURNING syntax) gets my huge patch reviewed. I'm still on the hook for parts of this one (and also for Pavel's date constructors stuff). I won't touch the ones that modify the core of ecpg, but I hope I hope I will be able to look at the other ones to ease work for Michael. I spent the last two weeks moving to another city, which was pretty exhausting, so I wasn't able to get as much done as I hoped. It's finally done now and today I got a stable network connection in the new place. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On 2013-10-21 09:58:30 -0700, Josh Berkus wrote: > Tom, > > >> If no-one really cares enough about a patch to review it, mark it > >> as "rejected, because no-one but the patch author cares". Harsh, > >> but that's effectively what pushing to the next commitfest means > >> anyway. > > > > Well, that could be the problem, but it's also possible that no one > > could get to it in the alloted CF timeframe. Maybe the > > best-qualified reviewers were on vacation, or maybe there were just > > too many patches. I could see bouncing a patch on this basis if it > > doesn't get touched for, say, two consecutive CFs. > > That would be more or less a declaration of failure by this project to > regulate our own development process, and an abandonment of the idea of > ever getting new contributors. If we don't guarantee legit patches at > least one review, why would anyone submit code to this project at all? Well, who are you going to get to review things that they consider simply bad ideas? I have no problem investing serious time in doing detailed reviews of patches I can see the point of, but reviews of stuff I think is pointless? Not really. > At some point folks on this list are going to admit that we have a > serious problem with reviews and reviewers, and that it's worth a > project-wide effort to do something about it. Apparently that day > hasn't come yet; most people are still in denial. The fact that people do agree with your solutions, doesn't imply that they don't care about the problem itself. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On 10/21/2013 06:56 AM, Heikki Linnakangas wrote: > I feel guilty to complain, while not actually volunteering to be a > commitfest manager myself, but I wish the commitfest manager would be > more aggressive in nagging, pinging and threatening people to review > stuff. If nothing else, always feel free to nag me :-). Josh tried that > with the infamous Slacker List, but that backfired. Rather than posting > a public list of shame, I think it would work better to send short > off-list nag emails, or chat via IM. Something like "Hey, you've signed > up to review this. Any progress?". Or "Hey, could you take a look at X > please? No-one else seems to care about it." Yeah, that doesn't work at all. It's been tried. Before I published the Slacker list, I emailed all of those folks privately (save one, due to an address typo), and 90% of them didn't even respond. Public shame, however reprehensible, was a vastly more effective motivator. Well, it works with *you*. But you were reviewing patches anyway. The simple problem is that, when it comes down to day-to-day work, our hackers collectively simply don't prioritize the review process. Years ago, people waited for Tom, Bruce, and Robert to review everything and went and did their own stuff. Now people are waiting for the CFM to organize everything, and go and do their own stuff. Either way, the majority of our contributors are dumping responsibility on someone else to see that review and commit happens, and that doesn't scale. Every single person who contributes to this project needs to take responsibility for making sure that patches get reviewed and committed, and worth some inconvenience to keep working. Until we do that, our review process will continue to be dysfunctional. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
On 10/21/2013 10:14 AM, Andres Freund wrote: > Well, who are you going to get to review things that they consider > simply bad ideas? I have no problem investing serious time in doing > detailed reviews of patches I can see the point of, but reviews of stuff > I think is pointless? Not really. That's still a review, if you actually do it. "I don't think this patch adds useful functionality because ..." >> At some point folks on this list are going to admit that we have a >> serious problem with reviews and reviewers, and that it's worth a >> project-wide effort to do something about it. Apparently that day >> hasn't come yet; most people are still in denial. > > The fact that people do agree with your solutions, doesn't imply that > they don't care about the problem itself. The fact that people don't propose of put any work into solutions of their own, while opposing solutions proposed by others, shows that they don't actually care. Where's your solution? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
On 2013-10-21 10:19:22 -0700, Josh Berkus wrote: > On 10/21/2013 10:14 AM, Andres Freund wrote: > > Well, who are you going to get to review things that they consider > > simply bad ideas? I have no problem investing serious time in doing > > detailed reviews of patches I can see the point of, but reviews of stuff > > I think is pointless? Not really. > > That's still a review, if you actually do it. "I don't think this patch > adds useful functionality because ..." Which people usually aren't happy enough with to accept their patch is refused. And usually you need a good amount of people disagreeing with something to make it go away. Those discussions usually take a good amount of energy. That many will prefer on something they see as productive. Like reviewing patches they see the point of. > >> At some point folks on this list are going to admit that we have a > >> serious problem with reviews and reviewers, and that it's worth a > >> project-wide effort to do something about it. Apparently that day > >> hasn't come yet; most people are still in denial. > > > > The fact that people do agree with your solutions, doesn't imply that > > they don't care about the problem itself. > > The fact that people don't propose of put any work into solutions of > their own, while opposing solutions proposed by others, shows that they > don't actually care. Where's your solution? I find it utterly ridiculous to accuse the people that *do* reviews of not doing anything. By doing code-level reviews reviewers teach authors and bystanders more about the code. Which actually can increase the number of review(ers) and even committers in the long run. And no, not having an own solution, doesn't turn somebody elses non-solution into a solution. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
2013-10-21 18:25 keltezéssel, Stephen Frost írta: > Zoltan, > > * Boszormenyi Zoltan (zb@cybertec.at) wrote: >> I even provided a repo @github where it was broken up into pieces that can be sanely reviewed. > You also gave the first person looking at the patch a hard time about > asking for it to be broken up; unnecessairly, imv. Sorry if it felt that way. I gave the commit IDs about the points he brought up. > Thanks for breaking > it up and for doing patch review of other patches- it does help the > project move forward. Try to be understanding when someone asks a > question that's already been answered; we're all quite busy and may > forget or miss things. > > I don't know if Alvaro will have time to look into this or if he perhaps > already has, but I had noticed this patch earlier and it was one of the > ones I had hoped to take a look at, as I've been in the ECPG bits due to > the work with Coverity that I've been doing. I'll try and provide > feedback later this week/weekend on it. > > Thanks, > > Stephen -- ---------------------------------- Zoltán Böszörményi Cybertec Schönig & Schönig GmbH Gröhrmühlgasse 26 A-2700 Wiener Neustadt, Austria Web: http://www.postgresql-support.de http://www.postgresql.at/
Andres, > I find it utterly ridiculous to accuse the people that *do* reviews of > not doing anything. By doing code-level reviews reviewers teach authors > and bystanders more about the code. Which actually can increase the > number of review(ers) and even committers in the long run. It would be nice if it worked that way -- in a review system which wasn't broken, it *should* work that way -- but a quick look at who's been doing the reviews for the last 2 releases shows that we no longer get new reviewers. Don't get me wrong. *you* are doing a ton of reviews, and if we had 20 people like you, then we wouldn't be talking about the review process at all because everything would be done already. But we don't. And, for that matter, I personally would love to see you not *need* to do so many reviews, so that you could spend more time working on LSR. > And no, not having an own solution, doesn't turn somebody elses > non-solution into a solution. Either you're proposing a solution, supporting someone else's solution, or you're saying the problem isn't important. There is no fourth alternative. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
2013-10-21 19:10 keltezéssel, Alvaro Herrera írta: > Boszormenyi Zoltan escribió: > >> I hoped that reviewing 4 patches in this CF (UNNEST, Extension templates, >> DISCARD SEQUENCES, and extended RETURNING syntax) gets my huge patch reviewed. > I'm still on the hook for parts of this one (and also for Pavel's date > constructors stuff). Thank you very much. > I won't touch the ones that modify the core of > ecpg, but I hope I hope I will be able to look at the other ones to ease > work for Michael. I hope he has time for this patch soon. When this patch was brought up last time, he expressed interest. > I spent the last two weeks moving to another city, > which was pretty exhausting, so I wasn't able to get as much done as I > hoped. It's finally done now and today I got a stable network > connection in the new place. I wish you the best with your new place and friendly neighbours! :-) Best regards, Zoltán Böszörményi -- ---------------------------------- Zoltán Böszörményi Cybertec Schönig & Schönig GmbH Gröhrmühlgasse 26 A-2700 Wiener Neustadt, Austria Web: http://www.postgresql-support.de http://www.postgresql.at/
<div class="moz-cite-prefix">On 22/10/13 02:56, Heikki Linnakangas wrote:<br /></div><blockquote cite="mid:52653289.2030207@vmware.com"type="cite">On 21.10.2013 16:15, Peter Eisentraut wrote: <br /><blockquote type="cite">On10/21/13 1:31 AM, Andres Freund wrote: <br /><blockquote type="cite">The point of the CF is exactly that all<br /> patches get at least one good round of review. Moving unreviewed patches <br /> to the next CF will let them justsuffer the same fate there. <br /></blockquote></blockquote><br /> Agreed. People have different views on what the purposeof a commitfest is, but IMO the point is to make sure that every patch submitted gets at least a cursory review ina timely fashion. Pushing patches to the next one because no-one has gotten around to review them is a failure. <br /><br/><blockquote type="cite">What is the alternative? <br /></blockquote><br /> If no-one really cares enough about a patchto review it, mark it as "rejected, because no-one but the patch author cares". Harsh, but that's effectively what pushingto the next commitfest means anyway. Better to be honest about it. At least that way the author can promote the patch'svirtues more on the mailing list, or personally contact someone who might be interested, to get some attention, andresubmit if he thinks that it might have a chance on the next commitfest. <br /><br /> Another alternative is to pushharder to make sure that every patch gets some review. I don't know how to accomplish that. Robert Haas did a great jobat that in the first few commitfests (IIRC), but only because he personally spent a lot of time not only managing thecommitfest but actually reviewing the patches that no-one else bothered with. That's a great way to make sure that everypatch gets some attention, but I don't think we have any takers for that role. <br /><br /> I feel guilty to complain,while not actually volunteering to be a commitfest manager myself, but I wish the commitfest manager would be moreaggressive in nagging, pinging and threatening people to review stuff. If nothing else, always feel free to nag me :-).Josh tried that with the infamous Slacker List, but that backfired. Rather than posting a public list of shame, I thinkit would work better to send short off-list nag emails, or chat via IM. Something like "Hey, you've signed up to reviewthis. Any progress?". Or "Hey, could you take a look at X please? No-one else seems to care about it." <br /><br />- Heikki <br /><br /><br /></blockquote> Hmm...<br /><br /> From at different area, but I think it may apply here...<br/><br /> When I was running a magazine for a computer user group, I regularly phoned people up to encourage themto write articles, I think I managed to get 50% of them to contribute articles.<br /><br /> In the pg context: this mightmean contacting patch submitters & potential reviewers, listening to their moans... and encouraging them. Sometimesthey may simply need some advice, or to be put in contact with someone who can explain something that is obscureto them - it might be a simple mental block, in someone that is otherwise extremely competent. A lot of this shouldbe done behind the scenes, the idea is more to empower than to shame (I'm sure that could be phrased better!). <br/><br /><br /> Cheers,<br /> Gavin<br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br/><br /><br /><br /><br /><br /><br /><br /><br />
On 10/21/13 9:18 AM, Andres Freund wrote: > I am not 100% sure, but what's the point of the CF if we're not actually > reviewing patches that wouldn't get review without it? So I guess it's > not starting the next one before we've finished - which we obviously > haven't in this case - the last one. The point is to get people to do some reviewing in the first place. If people don't want to review certain things or are exhausted after a month, extending the commitfest is not going to achieve much.
Josh Berkus <josh@agliodbs.com> writes:
> Either you're proposing a solution, supporting someone else's solution,
> or you're saying the problem isn't important. There is no fourth
> alternative.
Nonsense. Pointing out that a proposed solution isn't workable is not
saying that the problem isn't important. Or are you trying to establish
a rule that we can't complain about a proposal unless we have another one
to make? Sorry, I won't accept that.
regards, tom lane
On Mon, Oct 21, 2013 at 2:48 PM, Peter Eisentraut <peter_e@gmx.net> wrote: > On 10/21/13 9:18 AM, Andres Freund wrote: >> I am not 100% sure, but what's the point of the CF if we're not actually >> reviewing patches that wouldn't get review without it? So I guess it's >> not starting the next one before we've finished - which we obviously >> haven't in this case - the last one. > > The point is to get people to do some reviewing in the first place. If > people don't want to review certain things or are exhausted after a > month, extending the commitfest is not going to achieve much. I agree with that, but I agree with Andres, too. CommitFests are supposed to be time-bounded, and they're also supposed to get a certain amount of work done, and they're supposed to do it using all-volunteer labor. Guaranteeing all of those things simultaneously clearly isn't possible; and yet some past CommitFest managers have been far more successful at it than others. I think it's more of an art than a science. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 10/21/2013 11:59 AM, Tom Lane wrote: > Josh Berkus <josh@agliodbs.com> writes: >> Either you're proposing a solution, supporting someone else's solution, >> or you're saying the problem isn't important. There is no fourth >> alternative. > > Nonsense. Pointing out that a proposed solution isn't workable is not > saying that the problem isn't important. Or are you trying to establish > a rule that we can't complain about a proposal unless we have another one > to make? Sorry, I won't accept that. In some cases the other solution is "we need to search for a better solution". But if you say "the proposed solution is bad" without even proposing criteria for a better solution, then you are *de facto* saying that the problem isn't important, whether or not you would like ot pretend that you're saying something else. If a problem is important, then it is worth solving. If it's not worth solving, then it's not important. This is just as true of bugs in our process as it is of bugs in our code; if we release without fixing a bug, then we are making a concrete statement that the bug is not important. For the past two years, we've proceeded without fixing the bugs in our process, which is a material statement that most contributors don't feel that the process is buggy. Heck, the whole discussion about reviews got cut from the Developer's Meeting agenda this year, and if that's not a statement of unimportance, I don't know what is. When I came up with the idea of CommitFests they were supposed to be an incremental improvement for us to build on. Instead it's remained frozen in amber, and steadily becoming less and less effective. I've suggested a number of improvements and changes over the years, and largely been rewarded with denial, attacks, ridicule, and general sandbaggery. I'm done. If the community doesn't think there's a problem, then clearly I'm in error for proposing fixes. Not sure who you're going to get to do CF3, though. I'm not going to be CFM again, and I'm pretty sure nobody else wants the job either. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
Josh,
* Josh Berkus (josh@agliodbs.com) wrote:
> In some cases the other solution is "we need to search for a better
> solution". But if you say "the proposed solution is bad" without even
> proposing criteria for a better solution, then you are *de facto* saying
> that the problem isn't important, whether or not you would like ot
> pretend that you're saying something else. If a problem is important,
> then it is worth solving. If it's not worth solving, then it's not
> important.
Or you're simply saying that other things hold priority over this
particular problem. Sure, that makes it *less* important than other
things (if priority is your measure of importance, and it may or may not
be) but it is not the same to say that something is unimportant.
> This is just as true of bugs in our process as it is of bugs in our
> code; if we release without fixing a bug, then we are making a concrete
> statement that the bug is not important.
Or that the priority of the release is *more* important. Things are not
all either red-or-blue here (to use politically correct colors).
> For the past two years, we've
> proceeded without fixing the bugs in our process, which is a material
> statement that most contributors don't feel that the process is buggy.
It's not hard to imagine that developers might feel that bugs, code,
hacking, etc, are of a higher priority than very vaugue and extremely
challenging process 'bugs'.
> If the community doesn't think there's a
> problem, then clearly I'm in error for proposing fixes.
For my 2c, I agree that there's a problem, but I've got a ton of other
tasks, not to mention $dayjob that makes it unlikely that I'll find time
to come up with alternate proposals. I will also say that I feel that
this process is still an *improvement* over the previous process, which
is really the only other one we've actually tested. Perhaps that means
we should just try different things out and see if we can't build the
best process out there through supervised learning.
Thanks,
Stephen
On 10/21/2013 08:11 AM, Robert Haas wrote: > Supposedly, we have a policy that for each patch you submit, you ought > to review a patch. That right there ought to provide enough reviewers > for all the patches, but clearly it didn't. And I'm pretty sure that > some people (like me) looked at a lot MORE patches than they > themselves submitted. I think auditing who is not contributing in > that area and finding tactful ways to encourage them to contribute > would be a very useful service to the project. What if as part of the patch submission process you had to pick the patch you were going to review? If there are no patches to review, then we obviously don't have a problem. If there are patches to review then we are all set. I guess there is the problem of there only being patches that a submitter is not qualified to review but I find that miniscule as every person on this list (myself included) can do a cursory review (patch applies, docs are good, indentation is appropriate, works as advertised). The commitfest app would have to be modified for this but what do people think? Joshua D. Drake -- Command Prompt, Inc. - http://www.commandprompt.com/ 509-416-6579 PostgreSQL Support, Training, Professional Services and Development High Availability, Oracle Conversion, Postgres-XC, @cmdpromptinc For my dreams of your image that blossoms a rose in the deeps of my heart. - W.B. Yeats
On Tue, Oct 22, 2013 at 2:38 PM, Joshua D. Drake <jd@commandprompt.com> wrote: > > On 10/21/2013 08:11 AM, Robert Haas wrote: > >> Supposedly, we have a policy that for each patch you submit, you ought >> to review a patch. That right there ought to provide enough reviewers >> for all the patches, but clearly it didn't. And I'm pretty sure that >> some people (like me) looked at a lot MORE patches than they >> themselves submitted. I think auditing who is not contributing in >> that area and finding tactful ways to encourage them to contribute >> would be a very useful service to the project. > > > What if as part of the patch submission process you had to pick the patch > you were going to review? If there are no patches to review, then we > obviously don't have a problem. If there are patches to review then we are > all set. > if we are going to modify the CF app (not offering myself, and i'm not trying to bind anyone also) i would prefer to see a flag stating if number of reviews registered there are less than submitted patches. This could be a column just after the author of a patch, so people can give preference to patches of submitters that are also reviewing other people's patches. -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación Phone: +593 4 5107566 Cell: +593 987171157
On Tue, Oct 22, 2013 at 12:27:13PM -0700, Josh Berkus wrote: > When I came up with the idea of CommitFests they were supposed to be an > incremental improvement for us to build on. Instead it's remained > frozen in amber, and steadily becoming less and less effective. I've > suggested a number of improvements and changes over the years, and > largely been rewarded with denial, attacks, ridicule, and general > sandbaggery. I'm done. If the community doesn't think there's a > problem, then clearly I'm in error for proposing fixes. > > Not sure who you're going to get to do CF3, though. I'm not going to be > CFM again, and I'm pretty sure nobody else wants the job either. For what it's worth, I liked how you ran CF 2013-06. It proceeded better than any CF of the 9.3 development cycle. I can appreciate that it drained you, though; you tried new things, and your reward was lots of flak. Your innovations were 85% good; sadly, debate raged over the negative aspects only. Perhaps that arises from how we deal with code. An 85%-good patch can still wreak havoc in the field; closing that gap is essential. We say little about the correct aspects of a patch; it's usually a given that things not mentioned are satisfactory and have self-evident value. That's not such an effective discussion pattern when the topic is management strategies. Thanks, nm -- Noah Misch EnterpriseDB http://www.enterprisedb.com
On Wed, Oct 23, 2013 at 8:38 AM, Noah Misch <noah@leadboat.com> wrote: > On Tue, Oct 22, 2013 at 12:27:13PM -0700, Josh Berkus wrote: >> When I came up with the idea of CommitFests they were supposed to be an >> incremental improvement for us to build on. Instead it's remained >> frozen in amber, and steadily becoming less and less effective. I've >> suggested a number of improvements and changes over the years, and >> largely been rewarded with denial, attacks, ridicule, and general >> sandbaggery. I'm done. If the community doesn't think there's a >> problem, then clearly I'm in error for proposing fixes. >> >> Not sure who you're going to get to do CF3, though. I'm not going to be >> CFM again, and I'm pretty sure nobody else wants the job either. > > For what it's worth, I liked how you ran CF 2013-06. It proceeded better than > any CF of the 9.3 development cycle. I can appreciate that it drained you, > though; you tried new things, and your reward was lots of flak. Your > innovations were 85% good; sadly, debate raged over the negative aspects only. > Perhaps that arises from how we deal with code. An 85%-good patch can still > wreak havoc in the field; closing that gap is essential. We say little about > the correct aspects of a patch; it's usually a given that things not mentioned > are satisfactory and have self-evident value. That's not such an effective > discussion pattern when the topic is management strategies. I couldn't have said it better myself. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Mon, Oct 21, 2013 at 11:10:09AM -0400, Tom Lane wrote: > Heikki Linnakangas <hlinnakangas@vmware.com> writes: > > On 21.10.2013 16:15, Peter Eisentraut wrote: > >> What is the alternative? > > > If no-one really cares enough about a patch to review it, mark it as > > "rejected, because no-one but the patch author cares". Harsh, but that's > > effectively what pushing to the next commitfest means anyway. > > Well, that could be the problem, but it's also possible that no one could > get to it in the alloted CF timeframe. Maybe the best-qualified reviewers > were on vacation, or maybe there were just too many patches. I could see > bouncing a patch on this basis if it doesn't get touched for, say, two > consecutive CFs. Maybe it would help if patches which got punted from the last commitfest without review were marked up in some way (red, bold) in the commitfest app so reviewers are nudged to maybe consider picking them up first. Michael
On 10/23/2013 05:38 AM, Noah Misch wrote: > We say little about > the correct aspects of a patch; it's usually a given that things not mentioned > are satisfactory and have self-evident value. That's not such an effective > discussion pattern when the topic is management strategies. It's not an effective discussion pattern when dealing with new code contributors either, or even some old ones. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com