Re: [9.4 CF 1] The Commitfest Slacker List

On Wed, Jul 3, 2013 at 12:03 PM, Christopher Browne <cbbrowne(at)gmail(dot)com>wrote:

> On Wed, Jul 3, 2013 at 2:24 PM, Cédric Villemain <cedric(at)2ndquadrant(dot)com>wrote:
>
>> > Clearly I ticked off a bunch of people by publishing "the list". On the
>> > other hand, in the 5 days succeeding the post, more than a dozen
>> > additional people signed up to review patches, and we got some of the
>> > "ready for committer" patches cleared out -- something which nothing
>> > else I did, including dozens of private emails, general pleas to this
>> > mailing list, mails to the RRReviewers list, served to accomplish, in
>> > this or previous CFs.
>>
>> Others rules appeared, like the 5 days limit.
>>
>
The limit was previously 4 days (at least according to
http://wiki.postgresql.org/wiki/RRReviewers) but I think that that was
honored almost exclusively in the breach. I don't have a sense for how the
5 day limit is working. If it is working, great. If not, I would advocate
lengthening it--having a limit specified but not generally held to is
counterproductive. I know that I, and at least one other potential
reviewer, won't ask to be assigned a random patch because we have no
confidence we can do an adequate review of a random patch within a
contiguous 5 day window.

On the other hand, I could always just go to the open commitfest (rather
than the in progress one), pick something at random myself, and have up to
3 months to review it. I just don't do have the discipline to do that, at
least not often.

> To me it outlines that some are abusing the CF app and pushing there
>> useless
>> patches (not still ready or complete, WIP, ...
>
>
> Seems to me that "useless" overstates things, but it does seem fair to
> say that some patches are not sufficiently well prepared to be efficiently
> added into Postgres.
>

I think that there will always be contributions that need some hand-holding
of some form. Isn't that what "returned with feedback" is for?

>
> > So, as an experiment, call it a mixed result. I would like to have some
>> > other way to motivate reviewers than public shame. I'd like to have
>> > some positive motivations for reviewers, such as public recognition by
>> > our project and respect from hackers, but I'm doubting that those are
>> > actually going to happen, given the feedback I've gotten on this list to
>> > the idea.
>>
>> You're looking at a short term, big effect.
>> And long term ? Will people listed still be interested to participate in a
>> project which stamps people ?
>>
>> With or without review, it's a shame if people stop proposing patches
>> because
>> they are not sure to get time to review other things *in time*.
>
>
> Well, if the project is hampered by not being able to get *all* the
> changes that people imagine that they want to put in, then we have a
> real problem of needing a sort of "triage" to determine which changes
> will be accepted, and which will not.
>
> Perhaps we need an extra status in the CommitFest application, namely
> one that characterizes:
> Insufficiently Important To Warrant Review
>
> That's too long a term. Perhaps "Not Review-worthy" expresses it better?
>

I don't think that this would really be an improvement. Someone still has
to spend enough time looking at the patch to make the decision that it
falls into one of those categories. Having spent sufficient time to do
that, what they did is ipso facto a review, and they should just set it as
either rejected (not important or idea unworkable) or RwF (idea is
workable, but the given implementation is not).

Cheers,

Jeff