On 12/11/2014 08:51 PM, Josh Berkus wrote:
On 12/11/2014 09:22 AM, Heikki Linnakangas wrote:
I imagine that it's the same for everyone else. Many of the patches that
sit in the commitfest for weeks are patches that no-one really cares
much about. I'm not sure what to do about that. It would be harsh to
reject a patch just because no-one's gotten around to reviewing it, and
if we start doing that, it's not clear what the point of a commitfest is
any more.
So the "nobody cares" argument is manifestly based on false assumptions.
Are you contending that nobody cares about UPSERT or GROUPING SETS?
No. I was thinking of patches like "Add IF NOT EXISTS to CREATE TABLE AS
and CREATE MATERIALIZED VIEW", "event triggers: more DROP info", "Point
to polygon distance operator" and "pgcrypto: support PGP signatures".
And nobody was an exaggeration; clearly *someone* cares about those
things, or they wouldn't have written a patch in the first place. But
none of the committers care enough to pick them up. Even in the case
that someone reviews them, it's often not because the reviewer is
interested in the patch, it's just to help out with the process.
(Apologies to the authors of those patches; they were just the first few
that caught my eye)
There's also a chicken-and-egg problem there. Say that we started not
reviewing DW features because "nobody cares". Then the people who
contributed those features don't go on to become major contributors,
which means they won't review further DW patches. Which means that
we've just closed off an entire use-case for PostgreSQL. I'd think that
PostGIS would have taught us the value of the "nobody cares" fallacy.
Also, if we go back on the promise that "every patch gets a review",
then we're definitely headed towards no more new contributors. As Noah
said at one developer meeting (to paraphrase), one of the few things
which keeps contributors persisting through our baroque,
poorly-documented, excessively political contribution process is the
promise that they'll get a fair shake for their invested time. If we
drop that promise, we'll solve our workflow problem by cutting off the
flow of new patches entirely.
Yeah, there is that.
Perhaps we should change the process so that it is the patch author's
responsibility to find a reviewer, and a committer, for the patch. If
you can't cajole anyone to review your patch, it's a sign that no-one
cares about it, and the patch is rejected by default. Or put a quick
+1/-1 voting option to each patch in the commitfest, to get a quick
gauge of how the committers feel about it.
Again, that process would favor existing contributors and other folks
who know how to "work the Postgres community". It would be effectively
the same as hanging up a sign which says "no new contributors wanted".
It would also be dramatically increasing the amount of politics around
submitted patches, which take up far more time than the technical work.
I was thinking that by getting candid feedback that none of the existing
contributors are interested to look at your patch, the author could
revise the patch to garner more interest, or perhaps promise to review
someone else's patch in return. Right now, the patches just linger, and
the author doesn't know why, what's going to happen or what to do about it.
I will also point out that some of the existing senior committers, who
are the heaviest burdened under the existing system, have also been the
most resistant to any change in the system. You (Heikki) yourself
expressed a strong opposition to any attempt to recruit more reviewers.
Huh, I did? Can you elaborate?
So, given that you are inside the heart of the problem, do you have a
solution other than your proposal above that we simply stop accepting
new contributors? Or is that your solution? It would work, for some
definition of "work".
I don't have any good solutions, I'm afraid. It might help to ping the
reviewers who have signed up more often, to make the reviews to happen
more quickly. I did some nudge people in the August commitfest, but I
felt that it didn't actually achieve much. The most efficient way to
move the commitfest forward was to just review more patches myself.
That's one thought. Robert said the same thing about when he was the
commitfest manager; he just reviewed most the patches himself in the
end. And you mentioned that Tom used to review 70% of all incoming
patches. How about we make that official? It's the commitfest manager's
duty to review all the patches. He can recruit others if he can, but at
the end of the day he's expected to take a look at every patch, and
commit or return with some feedback.
The problem with that is that we'll have a hard time to find volunteers
for that. But we only need to find one sucker for each commitfest. I can
volunteer to do that once a year; if the other active committers do the
same, we're covered.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
