On Fri, Apr 6, 2012 at 3:22 AM, Greg Smith <g...@2ndquadrant.com> wrote: > On 04/05/2012 05:03 PM, Daniel Farina wrote: >> To get to the point, I wonder if it makes sense for someone who has a >> better sense a-priori what they're looking for in a committable patch >> (i.e. a committer, or someone with a telepathic link to one or more) >> to delegate specific pieces of patches for thorough review, sketching >> some thoughts about pitfalls or hazards to look for. Think of it as a >> patch checklist that is informed by the experience of a committer >> skimming its contents. > > It's tricky to add this sort of idea into the current patch workflow in all > cases. A decent percentage of submissions bounce off a reviewer who doesn't > commit, such that no committer spends time on them yet they still move > forward. Any idea that tries to involve committers earlier in the process > risks using more of their time on things that ultimately are never going to > pass toward commit anyway. That is after all where this whole process > started at, before CFs, and we know that didn't scale well. > > We already have a loose bar in this exact area, one that suggests larger > patches should first appear earlier in the development cycle. That happened > in this case, with a first submission in mid November, but it wasn't enough > to make things go smoothly. > > When I trace back how that played out, I think the gap in this case came > from not being sure who was going to commit the result at the end. If it's > early January, and the community knows there's a large feature approaching > because it's been around for over a month already, we better be asking who > is going to commit it eventually already. Many patch submitters agonize > over "which committer is going to grill me most?", and the bigger the patch > the more that impacts them. > > Nailing that down and putting together the sort of early committer checklist > you're describing strikes me as something that might happen in the first > week of a CF, before there are normally a large pile of "Ready for > Committer" things around. Raising these question--who is interesting in > committing big things and where they consider the bar to be--is something > submitters who have a stake in seeing their patches go on might do. > > It's better if people who are already working hard on features don't feel > like they need to wander around begging for someone to pay attention to them > though. Ideally it would be the sort of thing a proper CF manager would > highlight for them. Unfortunately we often don't quite have one of those, > instead just having people who sometimes make a bunch of noise at the > beginning of a CF and then go AWOL. (cough) No one is happy about that, and > it's something that clearly needs to be solved better too.
I think this basically just boils down to too many patches and not enough people. I was interested in Command Triggers from the beginning of this CommitFest, and I would have liked to pick it up sooner, but there were a LOT of patches to work on for this CommitFest. The first three CommitFests of this cycle each had between 52 and 60 patches, while this one had 106 which included several very complex and invasive patches, command triggers among them. So there was just a lot more to do, and a number of the people who submitted all of those patches didn't do a whole lot to help review them, sometimes because they were still furiously rewriting their submissions. It's not surprising that more patches + fewer reviewers = each patch getting less attention, or getting it later. Even before this CommitFest, it's felt to me like this hasn't been a great cycle for reviewing. I think we have generally had fewer people doing reviews than we did during the 9.0 and 9.1 cycles. I think we had a lot of momentum with the CommitFest process when it was new, but three years on I think there's been some ebbing of the relative enthusiastic volunteerism that got off the ground. I don't have a very good idea what to do about that, but I think it bears some thought. On a related note, letting CommitFests go on for three months because there's insufficient reviewer activity to get them done in one or two is, in my opinion, not much of a solution. If there's even less reviewer activity next time, are we going to let it go on for four months? Six months? Twelve months? At some point, it boils down to "we're just going to stop accepting patches for an indefinite period of time". Yuck. I also think we need to be generally more open to negative feedback than we are. Most if not all people who give negative feedback are not doing so because they wish to torpedo a given patch as because they have legitimate concerns which, since we are a community of smart and talented people here, are often well-founded. I've been there myself, and it's occasionally frustrating, but overall it leads to me doing better work than I otherwise would, which is a hard outcome to complain about when you get right down to it. When it's discovered and agreed that a patch has serious problems that can't be corrected in a few days, the response to that should be "crap, OK, see you next CommitFest". When people aren't willing to accept that, then we end up having arguments. Sure, there will sometimes be times when someone (often Tom, but sometimes another committer or some other experienced volunteer) will be able to step in and rewrite it enough to make it committable. But it seems to me that trying to minimize problems that are found by well-intentioned review for purposes of trying to squeeze something in under a deadline don't work out well. Either the feature goes in buggy, and we spend months trying to sort it out, or else we end up impugning the good will or technical judgement of the people who volunteer to review, which is not a good way to get the larger number of reviewers and CommitFest managers that we so desperately need. Whoever wins the argument, the community loses. I think that there are many projects that are more open to "just committing things" than we are - perhaps no better exemplified than by Tom's recent experiences with Henry Spencer's regular expression engine and the Tcl project. If you're willing to do work, we'll assume it's good; if you know something and are willing to do work, here are the keys. We could decide to take that approach, and just generally lower our standards for commit across the board. I am not in favor of that. I think the fact that we insist on good design and high-quality code is a real strength of this project and something that makes me proud to be associated with it. Yeah, it's harder that way. Yeah, sometimes it means that it takes longer before a given feature gets committed. Yeah, some things don't get done at all. But on the flip side, it means that when you use a PostgreSQL feature, you can pretty much count on it working. And if by some chance it doesn't, you can count on it being an oversight that will be corrected in the next maintenance release rather than a design problem that will never get fixed. I like that, and I think our users do too. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
