Re: Releasing in September - Mailing list pgsql-hackers

From Joel Jacobson
Subject Re: Releasing in September
Date
Msg-id CAASwCXfLbxR_LGdDQnmgbemc_+HKa2E+mULGHPB4MvfY47dzMQ@mail.gmail.com
Whole thread Raw
In response to Re: Releasing in September  (Andres Freund <andres@anarazel.de>)
Responses Re: Releasing in September  ("Joshua D. Drake" <jd@commandprompt.com>)
Re: Releasing in September  (Bruce Momjian <bruce@momjian.us>)
List pgsql-hackers
On Wed, Jan 20, 2016 at 5:29 PM, Andres Freund <andres@anarazel.de> wrote:
> I think one thing we should work on, is being absolutely religious about
> requiring, say, 2 reviews for every nontrivial contribution.  We
> currently seem to have a significantly increased submission rate, and at
> the same time the number of reviews per patch has gone down
> substantially.  I think the "honor" system has failed in that regard.

Good point, totally agree.

So the project should try to think of new ideas on how to incentivise reviewing?

I have three ideas so far:

1. The way I see it, the honor system is based on being mentioned
in the commit message and the release notes.

Authors are always mentioned in the release notes,
but I see reviewers are mostly only mentioned in the commit messages.

Maybe more skilled developers would think it's cool to do reviewing
if they were "paid" by also being mentioned in the release notes?

At least some skilled people would probably be motivated by it,
in addition to the good feeling of doing something just because it's
fun or important.

2. Currently "Needs review" can mean a patch is in any of the phases
(Submission -> Usability -> Feature test -> Performance review ->
Coding review -> Architecture review -> Review review).
If you as a reviewer don't even know if the patch will be accepted and
committed by a committer,
there is a risk review-time will be wasted until the patch has been
accepted by the community.

I suggest we inject a new intermediate optional step "Needs Code
Review" to mean the feature has been discussed on pghackers
and there is a consensus among committers that they at least agree the
feature is something desired,
and that someone has at least reviewed the patch applies and the
feature works like expected.
And maybe renaming the "Needs review" step to "Needs Usability Review"
(or some other word to capture the Submission -> Usability -> Feature
test -> Performance review, phase before the Code review phase).

Then you as a reviewer would be confident the feature will get
accepted as long as the code is correct,
and the time you spent on code-reviewing will not be wasted.

3. For those who are partly motivated by money,
maybe this idea would work too to some extent:

Since Trustly's Bug Country program [1] is aimed at motivating people
to find bugs in HEAD hasn't produced a single report so far,
maybe we should extend it to also cover reviews of commits
marked as "Ready For Committer" that shed light on a problem
that could have caused data corruption,
preventing a bad commit from being committed.

If we suddenly get hundreds of new reviewers who find hundreds of bugs
in uncommitted code, then maybe we have to change the terms again,
but I doubt neither of those will happen.

If people think this is a good idea, I can discuss internally if we can change
the conditions of the bug country program accordingly.

[1] https://trustly.com/se/postgresql-bug-bounty/



pgsql-hackers by date:

Previous
From: Kevin Grittner
Date:
Subject: Re: Proposal for UPDATE: do not insert new tuple on heap if update does not change data
Next
From: Tom Lane
Date:
Subject: Re: Releasing in September