Re: Kudos for Reviewers -- straw poll

Hackers,

I'd like to take a straw poll here on how we should acknowledge
reviewers. Please answer the below with your thoughts, either on-list
or via private email.

How should reviewers get credited in the release notes?

a) not at all
b) in a single block titled "Reviewers for this version" at the bottom.
c) on the patch they reviewed, for each patch

Should there be a criteria for a "creditable" review?

a) no, all reviews are worthwhile
b) yes, they have to do more than "it compiles"
c) yes, only code reviews should count

Should reviewers for 9.4 get a "prize", such as a t-shirt, as a
promotion to increase the number of non-submitter reviewers?

a) yes
b) no
c) yes, but submitters and committers should get it too

Thanks for your feedback!

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On 25.06.2013 20:17, Josh Berkus wrote:
> Hackers,
>
> I'd like to take a straw poll here on how we should acknowledge
> reviewers. Please answer the below with your thoughts, either on-list
> or via private email.
>
> How should reviewers get credited in the release notes?
>
> a) not at all
> b) in a single block titled "Reviewers for this version" at the bottom.
> c) on the patch they reviewed, for each patch

a)

Sometimes a reviewer contributes greatly to the patch, revising it and
rewriting parts of it. At that point, it's not just a review anymore,
and he/she should be mentioned in the release notes as a co-author.

> Should there be a criteria for a "creditable" review?
>
> a) no, all reviews are worthwhile
> b) yes, they have to do more than "it compiles"
> c) yes, only code reviews should count

This is one reason why I answered a) above. I don't want to set a criteria.

> Should reviewers for 9.4 get a "prize", such as a t-shirt, as a
> promotion to increase the number of non-submitter reviewers?
>
> a) yes
> b) no
> c) yes, but submitters and committers should get it too

a).

I don't think we should make any promises, though. Just arbitrarily send
a t-shirt when you feel that someone has done a good job reviewing other
people's patches. And I'm not sure it's really worth the trouble, to
arrange the logistics etc.

- Heikki

On Tue, June 25, 2013 19:17, Josh Berkus wrote:

> How should reviewers get credited in the release notes?
b) in a single block titled "Reviewers for this version" at the bottom.

> Should there be a criteria for a "creditable" review?
b) yes, they have to do more than "it compiles"

> Should reviewers for 9.4 get a "prize", such as a t-shirt, as a
> promotion to increase the number of non-submitter reviewers?
b) no

Erik Rijkers

On 2013-06-25 10:17:07 -0700, Josh Berkus wrote:
> How should reviewers get credited in the release notes?
>
> a) not at all
> b) in a single block titled "Reviewers for this version" at the bottom.
> c) on the patch they reviewed, for each patch

b).

If the review was substantial enough the reviewer gets bumped to a
secondary author, just as it already happens.

> Should there be a criteria for a "creditable" review?
>
> a) no, all reviews are worthwhile
> b) yes, they have to do more than "it compiles"
> c) yes, only code reviews should count

b). Surely performance reviews should also count, they can be at least
as time consuming as a code review, so c) doesn't seem to make sense.

> Should reviewers for 9.4 get a "prize", such as a t-shirt, as a
> promotion to increase the number of non-submitter reviewers?
>
> a) yes
> b) no
> c) yes, but submitters and committers should get it too

Not sure. Seems like it might be a way to spend a lot of effort without
achieving all that much. But I can also imagine that it feels nice and
encourages a casual reviewer/contributor.

So it's either b) or c). Although I'd perhaps exclude regular
contributors to keep the list reasonable?

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 06/25/2013 10:46 AM, Andres Freund wrote:
> Not sure. Seems like it might be a way to spend a lot of effort without
> achieving all that much. But I can also imagine that it feels nice and
> encourages a casual reviewer/contributor.
>
> So it's either b) or c). Although I'd perhaps exclude regular
> contributors to keep the list reasonable?

Well, one of the other "prizes" which occurred to me today would be a
pgCon lottery. That is, each review posted by a non-committer would go
in a hat, and in February we would draw one who would get a free
registration and airfare to pgCon.

Seems apropos and without the horrible logistics issues of mailing
tshirts to 15 countries.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On 06/25/2013 10:17 AM, Josh Berkus wrote:
>
> Hackers,
>
> I'd like to take a straw poll here on how we should acknowledge
> reviewers. Please answer the below with your thoughts, either on-list
> or via private email.
>
> How should reviewers get credited in the release notes?
>
> a) not at all
> b) in a single block titled "Reviewers for this version" at the bottom.
> c) on the patch they reviewed, for each patch

C. The idea that reviewers are somehow less than authors is rather
disheartening.

>
> Should there be a criteria for a "creditable" review?
>
> a) no, all reviews are worthwhile
> b) yes, they have to do more than "it compiles"
> c) yes, only code reviews should count

B. I think it compiles, and I tested it via X should be the minimum.
Here is a case. I was considering taking a review of the new Gin Cache
patch. I can't really do a "code" review but I can certainly run
benchmarking tests before/after and apply the patch etc.

>
> Should reviewers for 9.4 get a "prize", such as a t-shirt, as a
> promotion to increase the number of non-submitter reviewers?
>
> a) yes
> b) no
> c) yes, but submitters and committers should get it too
>
> Thanks for your feedback!
>

B. We already give them a multi-million dollar piece of software for free.

JD

--
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 2013-06-25 11:04:38 -0700, Joshua D. Drake wrote:
> >a) not at all
> >b) in a single block titled "Reviewers for this version" at the bottom.
> >c) on the patch they reviewed, for each patch
>
> C. The idea that reviewers are somehow less than authors is rather
> disheartening.

It's not about the reviewers being less. It's a comparison of
effort. The effort for a casual review simply isn't comparable with the
effort spent on developing a nontrivial patch.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 06/25/2013 01:17 PM, Josh Berkus wrote:
> Hackers,
>
> I'd like to take a straw poll here on how we should acknowledge
> reviewers. Please answer the below with your thoughts, either on-list
> or via private email.
>
> How should reviewers get credited in the release notes?
>
> a) not at all
> b) in a single block titled "Reviewers for this version" at the bottom.
> c) on the patch they reviewed, for each patch

b) seems about right.

>
> Should there be a criteria for a "creditable" review?
>
> a) no, all reviews are worthwhile
> b) yes, they have to do more than "it compiles"
> c) yes, only code reviews should count

c). Compilation, functionality and performance tests are useful, but
what we desperately need are in depth code reviews of large patches. If
we debase the currency by rewarding things less than that then any
incentive effect of kudos in encouraging more reviews is lost.

>
> Should reviewers for 9.4 get a "prize", such as a t-shirt, as a
> promotion to increase the number of non-submitter reviewers?
>
> a) yes
> b) no
> c) yes, but submitters and committers should get it too

I'd like to see prizes each release for "best contribution" and "best
reviewer" - I've thought for years something like this would be worth
trying. Committers and core members should not be eligible - this is
about encouraging new people.

cheers

andrew

On 06/25/2013 11:26 AM, Andres Freund wrote:
>
> On 2013-06-25 11:04:38 -0700, Joshua D. Drake wrote:
>>> a) not at all
>>> b) in a single block titled "Reviewers for this version" at the bottom.
>>> c) on the patch they reviewed, for each patch
>>
>> C. The idea that reviewers are somehow less than authors is rather
>> disheartening.
>
> It's not about the reviewers being less. It's a comparison of
> effort. The effort for a casual review simply isn't comparable with the
> effort spent on developing a nontrivial patch.

I think this is a backwards way to look at it.

The effort may not be comparable but the drudgery certainly is.

Reviewing patches sucks. Writing patches (for the most part) is fun.

Should the patch submitter get first billing? Yes.
Should the reviewer that makes sure to a reasonable level that the patch
is sane also get billing? Absolutely.
Should the reviewer get billing that is about the patch they reviewed. Yes.

As I mentioned before in the release notes something like:

Author: Tom Lane
Reviewer(s): Greg Stark, Andrew Dunstan

I think that is perfectly reasonable.

JD

--
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, Jun 25, 2013 at 2:17 PM, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
> How should reviewers get credited in the release notes?
> c) on the patch they reviewed, for each patch

This not only makes sense, it also lets people reading release notes
know there's been a review, and how thorough it was. I know, all
changes in PG get reviewed, but having it explicit on release notes I
imagine would be useful. Especially if I know the reviewers.

The co-author part also makes a lot of sense. When a reviewer
introduces changes directly, and they get committed, I think it should
be automatically considered co-authoring the patch.

> Should there be a criteria for a "creditable" review?
>
> a) no, all reviews are worthwhile

It's not that they're all worthwhile, but arbitrary decisions lend
themselves to arbitrary criticism. Whatever criteria should be
straightforward, unambiguous and unbiased, and it's hard getting all
those three in any other criteria than: all are worthwhile.

> b) yes, they have to do more than "it compiles"

This one's better than nothing, if you must have a minimum criteria.
But then people will just point out some trivial stuff and you'd be
tempted to say that trivialities don't count... and you get a snowball
going. IMHO, it's all or nothing.

> Should reviewers for 9.4 get a "prize", such as a t-shirt, as a
> promotion to increase the number of non-submitter reviewers?
>
> b) no

Yeah, while a fun idea, I don't think the logistics of it make it
worth the effort. Too much effort for too little benefit.

And I think recognition is a far better incentive than T-shirts
anyway. I know I'd be encouraged to review patches for the recognition
alone, a lot more than for a T-shirt I might not get. Contributing is
nice, but feeling appreciated while doing so is better.

On 06/25/2013 11:26 AM, Andres Freund wrote:
> On 2013-06-25 11:04:38 -0700, Joshua D. Drake wrote:
>>> a) not at all
>>> b) in a single block titled "Reviewers for this version" at the bottom.
>>> c) on the patch they reviewed, for each patch
>>
>> C. The idea that reviewers are somehow less than authors is rather
>> disheartening.
>
> It's not about the reviewers being less. It's a comparison of
> effort. The effort for a casual review simply isn't comparable with the
> effort spent on developing a nontrivial patch.

On the other hand, I will point out that we currently have a shortage of
reviewers, and we do NOT have a shortage of patch submitters. Seems
like we should apply incentives where we need help, no?

Mind you, my votes are (B), (A), and (A).

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On 25 June 2013 18:17, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
> Hackers,
>
> I'd like to take a straw poll here on how we should acknowledge
> reviewers. Please answer the below with your thoughts, either on-list
> or via private email.
>
> How should reviewers get credited in the release notes?
>
> a) not at all
> b) in a single block titled "Reviewers for this version" at the bottom.
> c) on the patch they reviewed, for each patch
>

b) Unless they contribute enough to the patch to be considered a co-author.

> Should there be a criteria for a "creditable" review?
>
> a) no, all reviews are worthwhile
> b) yes, they have to do more than "it compiles"
> c) yes, only code reviews should count
>

a) Sometimes even "it compiles" can be worthwhile, if there is doubt
over platform compatibility. All contributions should be acknowledged
and encouraged.

> Should reviewers for 9.4 get a "prize", such as a t-shirt, as a
> promotion to increase the number of non-submitter reviewers?
>
> a) yes
> b) no
> c) yes, but submitters and committers should get it too
>

b) Getting your name in the fine manual is reward enough ;-)

Regards,
Dean

On Tue, Jun 25, 2013 at 10:17:07AM -0700, Josh Berkus wrote:
> Hackers,
>
> I'd like to take a straw poll here on how we should acknowledge
> reviewers. Please answer the below with your thoughts, either on-list
> or via private email.
>
> How should reviewers get credited in the release notes?
>
> a) not at all
> b) in a single block titled "Reviewers for this version" at the bottom.
> c) on the patch they reviewed, for each patch

c) This keeps history better.

> Should there be a criteria for a "creditable" review?
>
> a) no, all reviews are worthwhile
> b) yes, they have to do more than "it compiles"
> c) yes, only code reviews should count

a). If it turns out that people are gaming this, or appear to be
gaming this, we can revisit the policy.

> Should reviewers for 9.4 get a "prize", such as a t-shirt, as a
> promotion to increase the number of non-submitter reviewers?
>
> a) yes
> b) no
> c) yes, but submitters and committers should get it too

b). You want to be *extremely* careful when paying volunteers. The
chances of damaging their intrinsic motivations are high, especially
when it's not stuff like covering their expenses.

http://www.iew.uzh.ch/wp/iewwp007.pdf

Cheers,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)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 26 June 2013 03:17, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
> How should reviewers get credited in the release notes?
>
> a) not at all
> b) in a single block titled "Reviewers for this version" at the bottom.
> c) on the patch they reviewed, for each patch

A weak preference for (c), with (b) running a close second. As others
have suggested, a review that leads to significant commitable changes
to the patch should bump the credit to co-authorship.

> Should there be a criteria for a "creditable" review?
>
> a) no, all reviews are worthwhile
> b) yes, they have to do more than "it compiles"
> c) yes, only code reviews should count

(b), the review should at least look at usabililty, doc, and
regression test criteria even if there is no in-depth code analysis.

> Should reviewers for 9.4 get a "prize", such as a t-shirt, as a
> promotion to increase the number of non-submitter reviewers?
>
> a) yes
> b) no
> c) yes, but submitters and committers should get it too

Provisionally (b), if we first try giving proper credit, and that
still doesn't drum up enough reviewing, then look to further incentive
schemes. No need to jump the gun.

Cheers,
BJ

Brendan Jurd wrote
> On 26 June 2013 03:17, Josh Berkus &lt;

> josh@

> &gt; wrote:
>> How should reviewers get credited in the release notes?
>>
>> a) not at all
>> b) in a single block titled "Reviewers for this version" at the bottom.
>> c) on the patch they reviewed, for each patch

I think some consideration toward a "commit and review summary" (outside the
release notes; and graphical/interactive in nature ideally) for each major
release is something worth considering. With regards to the release notes
I'd lean toward (b); significant contributions getting bumped to co-author
on specific patches covers (c) fairly well. I am unsure whether release
note mentions are significant enough motivation...see other thoughts below.

>> Should there be a criteria for a "creditable" review?
>>
>> a) no, all reviews are worthwhile
>> b) yes, they have to do more than "it compiles"
>> c) yes, only code reviews should count

Ideally (a) though (b) conceptually makes sense but it is too generic.

>> Should reviewers for 9.4 get a "prize", such as a t-shirt, as a
>> promotion to increase the number of non-submitter reviewers?
>>
>> a) yes
>> b) no
>> c) yes, but submitters and committers should get it too

One low-cost "prize" that I've pondered is, on an ongoing basis, the ability
to post a link and/or message to the PostgreSQL front page within a
significantly less stringent barrier to "acceptance" than is required for
current content. Basically except for topics or presentations deemed of
poor taste or detrimental to the project anything should be allowed. Some
kind of "this message was allowed because so-and-so has recently made the
following significant contributions to the project". There are probably
quite a few logistics to deal with down this path but a sponsor platform for
shameless self-promotion for people making the project successful -
something visible on an ongoing basis and not just once a year in a release
note - is likely a very valuable to the contributor while fairly inexpensive
to the project (i.e., some risk of reputation and some cost to setup the
infrastructure).

David J.

--
View this message in context: http://postgresql.1045698.n5.nabble.com/Kudos-for-Reviewers-straw-poll-tp5760952p5761031.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.

Dean Rasheed wrote:
>> How should reviewers get credited in the release notes?
>>
>> a) not at all
>> b) in a single block titled "Reviewers for this version" at the bottom.
>> c) on the patch they reviewed, for each patch
>>
>
> b) Unless they contribute enough to the patch to be considered a co-author.
>
>
>> Should there be a criteria for a "creditable" review?
>>
>> a) no, all reviews are worthwhile
>> b) yes, they have to do more than "it compiles"
>> c) yes, only code reviews should count
>>
>
> a) Sometimes even "it compiles" can be worthwhile, if there is doubt
> over platform compatibility. All contributions should be acknowledged
> and encouraged.
>
>
>> Should reviewers for 9.4 get a "prize", such as a t-shirt, as a
>> promotion to increase the number of non-submitter reviewers?
>>
>> a) yes
>> b) no
>> c) yes, but submitters and committers should get it too
>>
>
> b) Getting your name in the fine manual is reward enough ;-)

+1, except that I like Josh's idea about a free ticket to pgCon.

Yours,
Laurenz Albe

For me, B,B and another B works.

Regards,

Atri

--
Regards,

Atri
l'apprenant

On Wed, Jun 26, 2013 at 10:40:17AM +1000, Brendan Jurd wrote:
> On 26 June 2013 03:17, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
> > How should reviewers get credited in the release notes?
> >
> > a) not at all
> > b) in a single block titled "Reviewers for this version" at the bottom.
> > c) on the patch they reviewed, for each patch
>
> A weak preference for (c), with (b) running a close second. As others
> have suggested, a review that leads to significant commitable changes
> to the patch should bump the credit to co-authorship.

As a reminder, I tried a variant of C for 9.2 beta release notes, and
got lots of complaints, particularly because the line describing the
feature now had many more names on it.

In my opinion, adding reviewer names to each feature item might result
in the removal of all names from features.

A poll is nice for gauging interest, but many people who vote don't
understand the ramifications of what they are voting on.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

On 06/26/2013 09:14 AM, Bruce Momjian wrote:
> On Wed, Jun 26, 2013 at 10:40:17AM +1000, Brendan Jurd wrote:
>> On 26 June 2013 03:17, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
>>> How should reviewers get credited in the release notes?
>>>
>>> a) not at all
>>> b) in a single block titled "Reviewers for this version" at the bottom.
>>> c) on the patch they reviewed, for each patch
>> A weak preference for (c), with (b) running a close second. As others
>> have suggested, a review that leads to significant commitable changes
>> to the patch should bump the credit to co-authorship.
> As a reminder, I tried a variant of C for 9.2 beta release notes, and
> got lots of complaints, particularly because the line describing the
> feature now had many more names on it.
>
> In my opinion, adding reviewer names to each feature item might result
> in the removal of all names from features.
>
> A poll is nice for gauging interest, but many people who vote don't
> understand the ramifications of what they are voting on.
>

That's why I voted for b :-)

cheers

andrew

On 06/25/2013 08:26 PM, Andres Freund wrote:
> It's not about the reviewers being less. It's a comparison of
> effort. The effort for a casual review simply isn't comparable with the
> effort spent on developing a nontrivial patch.

Remember: "Debugging is twice as hard as writing the code in the first
place. ..." (Brian Kernighan)

IMO, the kind of reviews we need are of almost "debug level" difficulty.
(To the point where the reviewer becomes a co-author or even takes over
and submits a completely revamped patch instead.)

I agree that the casual review is several levels below that, so your
point holds. I doubt we need more reviews of that kind, though.

Thus, I'm in the AAB camp. The remaining question being: What's the
criterion for becoming a co-author (and thus getting mentioned in the
release notes)?

If at all, we should honor quality work with a "prize". Maybe a price
for the best reviewer per CF? Maybe even based on votes from the general
public on who's been the best, so reviews gain attention that way...
"Click here to vote for my review." ... Maybe a crazy idea.

Regards

Markus Wanner

Josh Berkus <josh(at)agliodbs(dot)com> writes:
> Well, one of the other "prizes" which occurred to me today would be a
> pgCon lottery. That is, each review posted by a non-committer would go
> in a hat, and in February we would draw one who would get a free
> registration and airfare to pgCon.

+1, I like that idea!

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

On Wed, Jun 26, 2013 at 10:25 AM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>
> On 06/26/2013 09:14 AM, Bruce Momjian wrote:
>>
>> On Wed, Jun 26, 2013 at 10:40:17AM +1000, Brendan Jurd wrote:
>>>
>>> On 26 June 2013 03:17, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
>>>>
>>>> How should reviewers get credited in the release notes?
>>>>
>>>> a) not at all
>>>> b) in a single block titled "Reviewers for this version" at the bottom.
>>>> c) on the patch they reviewed, for each patch
>>>
>>> A weak preference for (c), with (b) running a close second. As others
>>> have suggested, a review that leads to significant commitable changes
>>> to the patch should bump the credit to co-authorship.
>>
>> As a reminder, I tried a variant of C for 9.2 beta release notes, and
>> got lots of complaints, particularly because the line describing the
>> feature now had many more names on it.
>>
>> In my opinion, adding reviewer names to each feature item might result
>> in the removal of all names from features.
>>
>> A poll is nice for gauging interest, but many people who vote don't
>> understand the ramifications of what they are voting on.
>>
>
>
> That's why I voted for b :-)

Yeah, with that in mind, I'd also switch to b.

I wouldn't complain, but if it's been tried and failed... what can I say?

On Wed, 26 Jun 2013 09:14:07 -0400
Bruce Momjian <bruce(at)momjian(dot)us> wrote:

> On Wed, Jun 26, 2013 at 10:40:17AM +1000, Brendan Jurd wrote:
> > On 26 June 2013 03:17, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
> > > How should reviewers get credited in the release notes?
> > >
> > > a) not at all
> > > b) in a single block titled "Reviewers for this version" at the
> > > bottom. c) on the patch they reviewed, for each patch
> >
> > A weak preference for (c), with (b) running a close second. As
> > others have suggested, a review that leads to significant
> > commitable changes to the patch should bump the credit to
> > co-authorship.
>
> As a reminder, I tried a variant of C for 9.2 beta release notes, and
> got lots of complaints, particularly because the line describing the
> feature now had many more names on it.

I am just someone that is thinking that maybe can review things...I am
not voting OK but I have a comment about your last email...
If people thinks (and with people I am not talking about myself but
regular committers and reviewers) think that option c is good, I think
that we should change the tool (or the way) that release notes are
done....I mean (and excuse my poor English) if people thing that it is
the way to go, we should make tools good enough for what people want
not change people thoughts cause tools are not good enough

>
> In my opinion, adding reviewer names to each feature item might result
> in the removal of all names from features.

Let's fix the way that release notes are done

>
> A poll is nice for gauging interest, but many people who vote don't
> understand the ramifications of what they are voting on.
>

I agree, but cost-benefit is what we should see here not just cost....

On Wed, Jun 26, 2013 at 03:12:00PM -0300, Rodrigo Gonzalez wrote:
> On Wed, 26 Jun 2013 09:14:07 -0400
> Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>
> > On Wed, Jun 26, 2013 at 10:40:17AM +1000, Brendan Jurd wrote:
> > > On 26 June 2013 03:17, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
> > > > How should reviewers get credited in the release notes?
> > > >
> > > > a) not at all
> > > > b) in a single block titled "Reviewers for this version" at the
> > > > bottom. c) on the patch they reviewed, for each patch
> > >
> > > A weak preference for (c), with (b) running a close second. As
> > > others have suggested, a review that leads to significant
> > > commitable changes to the patch should bump the credit to
> > > co-authorship.
> >
> > As a reminder, I tried a variant of C for 9.2 beta release notes, and
> > got lots of complaints, particularly because the line describing the
> > feature now had many more names on it.
>
> I am just someone that is thinking that maybe can review things...I am
> not voting OK but I have a comment about your last email...
> If people thinks (and with people I am not talking about myself but
> regular committers and reviewers) think that option c is good, I think
> that we should change the tool (or the way) that release notes are
> done....I mean (and excuse my poor English) if people thing that it is
> the way to go, we should make tools good enough for what people want
> not change people thoughts cause tools are not good enough

Production of the release notes was not the problem; it was the text in
the release notes. I don't see how we could modify the release note
format.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

On Wed, 26 Jun 2013 14:13:32 -0400
Bruce Momjian <bruce(at)momjian(dot)us> wrote:

> On Wed, Jun 26, 2013 at 03:12:00PM -0300, Rodrigo Gonzalez wrote:
> > On Wed, 26 Jun 2013 09:14:07 -0400
> > Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> >
> > > On Wed, Jun 26, 2013 at 10:40:17AM +1000, Brendan Jurd wrote:
> > > > On 26 June 2013 03:17, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
> > > > > How should reviewers get credited in the release notes?
> > > > >
> > > > > a) not at all
> > > > > b) in a single block titled "Reviewers for this version" at
> > > > > the bottom. c) on the patch they reviewed, for each patch
> > > >
> > > > A weak preference for (c), with (b) running a close second. As
> > > > others have suggested, a review that leads to significant
> > > > commitable changes to the patch should bump the credit to
> > > > co-authorship.
> > >
> > > As a reminder, I tried a variant of C for 9.2 beta release notes,
> > > and got lots of complaints, particularly because the line
> > > describing the feature now had many more names on it.
> >
> > I am just someone that is thinking that maybe can review things...I
> > am not voting OK but I have a comment about your last email...
> > If people thinks (and with people I am not talking about myself but
> > regular committers and reviewers) think that option c is good, I
> > think that we should change the tool (or the way) that release
> > notes are done....I mean (and excuse my poor English) if people
> > thing that it is the way to go, we should make tools good enough
> > for what people want not change people thoughts cause tools are not
> > good enough
>
> Production of the release notes was not the problem; it was the text
> in the release notes. I don't see how we could modify the release
> note format.
>

Well...

Checking release notes for 9.2.4

you have Fix insecure parsing of server command-line switches
(Mitsumasa Kondo, Kyotaro Horiguchi)

What about (it people think that it is good) a second () with reviewed
by <someone>....

On Wed, Jun 26, 2013 at 03:22:06PM -0300, Rodrigo Gonzalez wrote:
> On Wed, 26 Jun 2013 14:13:32 -0400
> > Production of the release notes was not the problem; it was the text
> > in the release notes. I don't see how we could modify the release
> > note format.
> >
>
> Well...
>
> Checking release notes for 9.2.4
>
> you have Fix insecure parsing of server command-line switches
> (Mitsumasa Kondo, Kyotaro Horiguchi)
>
> What about (it people think that it is good) a second () with reviewed
> by <someone>....

That's what we had, and people didn't like it. If we overload that list
of names, we might find we want to remove all the names.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

Bruce Momjian escribió:
> On Wed, Jun 26, 2013 at 03:22:06PM -0300, Rodrigo Gonzalez wrote:

> > Checking release notes for 9.2.4
> >
> > you have Fix insecure parsing of server command-line switches
> > (Mitsumasa Kondo, Kyotaro Horiguchi)
> >
> > What about (it people think that it is good) a second () with reviewed
> > by <someone>....
>
> That's what we had, and people didn't like it. If we overload that list
> of names, we might find we want to remove all the names.

Yeah, it becomes too long. (For security patches, in particular, it's
probably not wise to list reviewers; there might well be reviewers whose
input you never see because they happened in the closed security list).

See the entry for foreign key locks:

Prevent non-key-field row updates from locking foreign key rows (Álvaro
Herrera, Noah Misch, Andres Freund, Alexander Shulgin, Marti Raudsepp)

I am the author of most of the code, yet I chose to add Noah and Andres
because they contributed a huge amount of time to reviewing the patch,
and Alex and Marti because they submitted some code. They are all
listed as coauthors, which seems a reasonable compromise to me.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 06/26/2013 12:02 PM, Alvaro Herrera wrote:
> See the entry for foreign key locks:
>
> Prevent non-key-field row updates from locking foreign key rows (Álvaro
> Herrera, Noah Misch, Andres Freund, Alexander Shulgin, Marti Raudsepp)
>
> I am the author of most of the code, yet I chose to add Noah and Andres
> because they contributed a huge amount of time to reviewing the patch,
> and Alex and Marti because they submitted some code. They are all
> listed as coauthors, which seems a reasonable compromise to me.

What about the idea that reviewers who do code revision work, like in
your FK patch, get listed after the original patch author with the
patch, and reviewers do more lightweight reviews get listed at the
bottom of the release notes? Seems fair to me.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On Tue, Jun 25, 2013 at 10:17 AM, Josh Berkus <josh(at)agliodbs(dot)com> wrote:

> How should reviewers get credited in the release notes?

Without getting into how we do this, I thought it might be helpful to
share the reasons why I believe recognizing and expressing gratitude
to reviewers is a helpful, useful and gratifying exercise for the
Postgres community.

I support crediting reviewers in a more formal way than we currently
do for a few different reasons.

First, I believe it's worth finding a way to say "Hey, you just did
something great for Postgres", publicly, to a bunch of people who
could have spent their valuable time and energy in some other way.

Second, reviewers get better at their work by reviewing multiple times
- so I'd like to encourage people to review more than once.

Third, reviewers don't always need to be expert developers, or experts
at Postgres to get started, but many people who do open source work
have no idea this is true. Public recognition helps make it clear that
we have people who give useful reviews and are relative novices.

We also have several different kinds of reviews:

* "does it compile"
* style/typo/easily seen bug passes
* in-depth discussion of design choices, use case, interface
* complex testing cases
* performance testing
* pre-commit checks for more subtle bugs or committer preferences.

All of those, except probably the very last, can be done by people who
are familiar with Postgres or its configuration, but aren't
necessarily Postgres or C experts.

Fourth, we have very few accepted ways to recognize contributions to
Postgres. Naming in Release Notes is one way this community has
consistently supported as a *public* way to say "hey, you just did
something great for Postgres". The complete list of ways I'm aware of
are:

1. Recognizing major, minor and emeritus contributors
2. Making someone a committer
3. Being part of the -core group
4. Naming authors by name in commit messages (but without consistent
metadata, making it difficult to count well)
5. Naming authors in release notes

That's pretty much it. That's great for the people who have already
secured positions through seniority, or because they're amazing C
hackers. I don't know if I need to lay out for everyone the value of
public recognition - if you want me to I can enumerate them. But the
benefits of public recognition are huge -- both in a social and a
financial sense.

Currently, the only way I know of to be recognized for work on
Postgres that is *not* seniority or code-related is #1. If you're a
reviewer, there's almost no chance you'll be recognized in that way
without some additional, very significant contribution to our
community. (Please let me know if I'm mistaken about this -- I only
know what I know!) Adding names to Release Notes (or some variant of
Release Notes) seems like a minor concession for work done that we as
a community value and want to encourage.

We are so few in terms of patch contributors - somewhere between
300-400 people contribute code to PostgreSQL each year based on the
names I try to pull out of commits. I haven't counted how many
reviewers we have who do not also contribute code.

Giving people appreciation for the review work they're doing for this
community, for free, is a good thing for everyone. Naming more names
helps describe the true scope of our community. Spreading gratitude is
good for those who thank and those who receive thanks (like, proven
scientifically!). And we increase the number of people who benefit
directly from the work that they do here, by giving them something
they can point their boss, their company and their colleagues to.

So, when we're debating *how* recognition might be done, please don't
lose sight of *why* this is important in the first place.

-selena

--
http://chesnok.com

On Thu, Jun 27, 2013 at 1:15 AM, Dimitri Fontaine
<dimitri(at)2ndquadrant(dot)fr> wrote:
> Josh Berkus <josh(at)agliodbs(dot)com> writes:
>> Well, one of the other "prizes" which occurred to me today would be a
>> pgCon lottery. That is, each review posted by a non-committer would go
>> in a hat, and in February we would draw one who would get a free
>> registration and airfare to pgCon.
>
> +1, I like that idea!
+1 on that also, looks like an excellent idea.
--
Michael

On 27/06/13 07:12, Josh Berkus wrote:
> On 06/26/2013 12:02 PM, Alvaro Herrera wrote:
>> See the entry for foreign key locks:
>>
>> Prevent non-key-field row updates from locking foreign key rows (Álvaro
>> Herrera, Noah Misch, Andres Freund, Alexander Shulgin, Marti Raudsepp)
>>
>> I am the author of most of the code, yet I chose to add Noah and Andres
>> because they contributed a huge amount of time to reviewing the patch,
>> and Alex and Marti because they submitted some code. They are all
>> listed as coauthors, which seems a reasonable compromise to me.
> What about the idea that reviewers who do code revision work, like in
> your FK patch, get listed after the original patch author with the
> patch, and reviewers do more lightweight reviews get listed at the
> bottom of the release notes? Seems fair to me.
>

I note reviewers are usually (?) mentioned in the commits to the git
repo - so maybe it is enough to list them at the bottom of the release
notes only?

Regards

Mark

On Tue, Jun 25, 2013 at 5:40 PM, Brendan Jurd <direvus(at)gmail(dot)com> wrote:

> On 26 June 2013 03:17, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
> > How should reviewers get credited in the release notes?
> >
> > a) not at all
> > b) in a single block titled "Reviewers for this version" at the bottom.
> > c) on the patch they reviewed, for each patch
>
> A weak preference for (c), with (b) running a close second. As others
> have suggested, a review that leads to significant commitable changes
> to the patch should bump the credit to co-authorship.
>

I like the "single block at the bottom" myself, with the same provision to
move a reviewer up to co-author.

>
> > Should there be a criteria for a "creditable" review?
> >
> > a) no, all reviews are worthwhile
> > b) yes, they have to do more than "it compiles"
> > c) yes, only code reviews should count
>
> (b), the review should at least look at usabililty, doc, and
> regression test criteria even if there is no in-depth code analysis.
>

+1 to this.

>
> > Should reviewers for 9.4 get a "prize", such as a t-shirt, as a
> > promotion to increase the number of non-submitter reviewers?
> >
> > a) yes
> > b) no
> > c) yes, but submitters and committers should get it too
>

I was going to go with b until I saw the suggestion for a PgCon ticket. I
really like that idea.

gabrielle

On Tue, Jun 25, 2013 at 2:27 PM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
> I'd like to see prizes each release for "best contribution" and "best
> reviewer" - I've thought for years something like this would be worth
> trying. Committers and core members should not be eligible - this is about
> encouraging new people.

Encouraging new people is good, but recognizing sustained, long-term
contributions is good, too. I think we should do more of that, too.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Thu, Jun 27, 2013 at 10:37 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Tue, Jun 25, 2013 at 2:27 PM, Andrew Dunstan <andrew(at)dunslane(dot)net>
> wrote:
> > I'd like to see prizes each release for "best contribution" and "best
> > reviewer" - I've thought for years something like this would be worth
> > trying. Committers and core members should not be eligible - this is
> about
> > encouraging new people.
>
> Encouraging new people is good, but recognizing sustained, long-term
> contributions is good, too. I think we should do more of that, too.
>

Conforming with David Fetter's pointer to the notion that sometimes attempts
to reward can backfire, I'm not sure that it will be super-helpful to
create "special"
rewards.

On the other hand, to recognize reviewer contributions in places relevant
to where
they take place seems pretty apropos, which could include:

a) Obviously we already capture this in the CommitFest web site (but it's
worth mentioning when trying to do a "census")

b) It would be a pretty good thing to mention reviewers within commit notes;
that provides some direct trace-back as to who it was that either validated
that the change was good, or that let a bad one slip through.

c) The release notes indicate authors of changes; to have a list of
reviewers
would be a fine thing.

If it requires inordinate effort to get the reviewers directly attached to
each
and every change, perhaps it isn't worthwhile to go to extreme efforts to
that
end.

It could be pretty satisfactory to have a simple listing, in the release
notes,
of the set of reviewers. That's a lot less bookkeeping than tracking this
for
each and every change.

The statement of such a list is a public acknowledgement of those that help
assure that the quality of PostgreSQL code remains excellent. (And that may
represent a good way to sell this "kudo".)

This allows organizations that are sponsoring PostgreSQL development to
have an extra metric by which *they* can recognize that their staff that
do such work are being recognized as contributors. It seems to me that
this is way more useful than a free t-shirt or the like.

On Thu, Jun 27, 2013 at 11:50:07AM -0400, Christopher Browne wrote:
> b) It would be a pretty good thing to mention reviewers within commit notes;
> that provides some direct trace-back as to who it was that either validated
> that the change was good, or that let a bad one slip through.
>
> c) The release notes indicate authors of changes; to have a list of reviewers
> would be a fine thing.
>
> If it requires inordinate effort to get the reviewers directly attached to each
> and every change, perhaps it isn't worthwhile to go to extreme efforts to that
> end.
>
> It could be pretty satisfactory to have a simple listing, in the release notes,
> of the set of reviewers. That's a lot less bookkeeping than tracking this for
> each and every change.

Adding the names to each release note item is not a problem; the
problem is the volume of names that overwhelms the release note text. If
we went that direction, I predict we would just remove _all_ names from
the release notes.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> On Thu, Jun 27, 2013 at 11:50:07AM -0400, Christopher Browne wrote:
>> It could be pretty satisfactory to have a simple listing, in the
>> release notes, of the set of reviewers. That's a lot less
>> bookkeeping than tracking this for each and every change.

> Adding the names to each release note item is not a problem; the
> problem is the volume of names that overwhelms the release note text. If
> we went that direction, I predict we would just remove _all_ names from
> the release notes.

Yeah. Keep in mind that the overwhelming majority of the audience for
the release notes doesn't actually give a darn who implemented what.

regards, tom lane

On 06/27/2013 12:12 PM, Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
>> On Thu, Jun 27, 2013 at 11:50:07AM -0400, Christopher Browne wrote:
>>> It could be pretty satisfactory to have a simple listing, in the
>>> release notes, of the set of reviewers. That's a lot less
>>> bookkeeping than tracking this for each and every change.
>> Adding the names to each release note item is not a problem; the
>> problem is the volume of names that overwhelms the release note text. If
>> we went that direction, I predict we would just remove _all_ names from
>> the release notes.
> Yeah. Keep in mind that the overwhelming majority of the audience for
> the release notes doesn't actually give a darn who implemented what.

Maybe we should have a Kudos / Bragging rights wiki page, with a table
something like this:

Release
Feature Name
Principal Author(s)
Contributing Author(s)
Code Reviewer(s)
Tester(s)

Constructing it going backwards would be an interesting task :-)

cheers

andrew

On 6/25/13 2:44 PM, Josh Berkus wrote:
> On the other hand, I will point out that we currently have a shortage of
> reviewers, and we do NOT have a shortage of patch submitters.

That's because reviewing is harder than initial development. The only
people who think otherwise are developers who don't do enough review.

--
Greg Smith 2ndQuadrant US greg(at)2ndQuadrant(dot)com Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.com

On 06/27/2013 08:56 AM, Bruce Momjian wrote:> Adding the names to each
release note item is not a problem; the
> problem is the volume of names that overwhelms the release note text. If
> we went that direction, I predict we would just remove _all_ names from
> the release notes.

That's not a realistic objection. We'd be talking about one or two more
names per patch, with a handful of exceptions.

If you're going to make an objection, object to the amount of extra work
it would be, which is signigficant with the current state of our
technology. Realistically, it'll take the help of additional people.

On 06/27/2013 09:19 AM, Tom Lane wrote:
> Yeah. Keep in mind that the overwhelming majority of the audience for
> the release notes doesn't actually give a darn who implemented what.

There is some argument to be made about moving the whole "list of names"
to some other resource, like a web page. In fact, if the mythical land
where we have a new CF application, that other resource could even be
generated by the CF app with some hand-tweaking (this would also require
some tweaks to community profiles etc.).

What I would be opposed to is continuing to list the original authors in
the release notes and putting reviewers, testers, co-authors, etc. on a
separate web page. If we're gonna move people, let's move *all* of
them. Also, it needs to be on something more trustworthy than the wiki,
like maybe putting it at postgresql.org/developers/9.3/

On Tue, Jun 25, 2013 at 2:27 PM, Andrew Dunstan <andrew(at)dunslane(dot)net>
wrote:
> Encouraging new people is good, but recognizing sustained, long-term
> contributions is good, too. I think we should do more of that, too.

I wasn't thinking about doing it every year -- just for 9.3, in order to
encourage more reviewers, and encourage reviewers to do more reviews.
But that would only work if we're also giving reviewers credit; as
several people have said, they care more about credit than they do about
prizes.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On Thu, Jun 27, 2013 at 10:10:23AM -0700, Josh Berkus wrote:
> On 06/27/2013 08:56 AM, Bruce Momjian wrote:> Adding the names to each
> release note item is not a problem; the
> > problem is the volume of names that overwhelms the release note text. If
> > we went that direction, I predict we would just remove _all_ names from
> > the release notes.
>
> That's not a realistic objection. We'd be talking about one or two more
> names per patch, with a handful of exceptions.

It is realistic because I did this for 9.2 beta and people didn't like
it; how much more realistic do you want?

> If you're going to make an objection, object to the amount of extra work
> it would be, which is significant with the current state of our
> technology. Realistically, it'll take the help of additional people.

No extra work required, it is all copy/paste for me.

> On 06/27/2013 09:19 AM, Tom Lane wrote:
> > Yeah. Keep in mind that the overwhelming majority of the audience for
> > the release notes doesn't actually give a darn who implemented what.
>
> There is some argument to be made about moving the whole "list of names"
> to some other resource, like a web page. In fact, if the mythical land
> where we have a new CF application, that other resource could even be
> generated by the CF app with some hand-tweaking (this would also require
> some tweaks to community profiles etc.).

Yes, I am fine with moving all names. However, I predict you will do
more to demotivate patch submitters than you will to motivate patch
reviewers. That is fine with me, as motivating developers/reviewers is
not our top priority.

> What I would be opposed to is continuing to list the original authors in
> the release notes and putting reviewers, testers, co-authors, etc. on a
> separate web page. If we're gonna move people, let's move *all* of
> them. Also, it needs to be on something more trustworthy than the wiki,
> like maybe putting it at postgresql.org/developers/9.3/

I think you will have trouble keeping those two lists synchronized. I
think you will need to create a release note document that includes all
names, then copy that to a website and remove the names just before the
release is packaged.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> On Thu, Jun 27, 2013 at 10:10:23AM -0700, Josh Berkus wrote:
>> What I would be opposed to is continuing to list the original authors in
>> the release notes and putting reviewers, testers, co-authors, etc. on a
>> separate web page. If we're gonna move people, let's move *all* of
>> them. Also, it needs to be on something more trustworthy than the wiki,
>> like maybe putting it at postgresql.org/developers/9.3/

> I think you will have trouble keeping those two lists synchronized. I
> think you will need to create a release note document that includes all
> names, then copy that to a website and remove the names just before the
> release is packaged.

Unless Bruce is doing more work than I think he is, the attribution data
going into the release notes is just coming from whatever the committer
said in the commit log message. I believe that we've generally been
careful to credit the patch author, but I'm less confident that everyone
who merited a "review credit" always gets mentioned --- that would
require going through the entire review thread at commit time, and I for
one can't say that I do that.

If we're going to try harder to ensure that reviewers are credited,
it'd probably be better to take both the commit log and the release
notes out of that loop.

regards, tom lane

> If we're going to try harder to ensure that reviewers are credited,
> it'd probably be better to take both the commit log and the release
> notes out of that loop.

I'd pull the reviewers out of the CF app. Even in its current
implementation, that's the easiest route.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On 06/27/2013 02:38 PM, Josh Berkus wrote:
>> If we're going to try harder to ensure that reviewers are credited,
>> it'd probably be better to take both the commit log and the release
>> notes out of that loop.
> I'd pull the reviewers out of the CF app. Even in its current
> implementation, that's the easiest route.
>

I have not honestly found these to be terribly accurate.

cheers

andrew

On Thu, Jun 27, 2013 at 02:17:25PM -0400, Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > On Thu, Jun 27, 2013 at 10:10:23AM -0700, Josh Berkus wrote:
> >> What I would be opposed to is continuing to list the original authors in
> >> the release notes and putting reviewers, testers, co-authors, etc. on a
> >> separate web page. If we're gonna move people, let's move *all* of
> >> them. Also, it needs to be on something more trustworthy than the wiki,
> >> like maybe putting it at postgresql.org/developers/9.3/
>
> > I think you will have trouble keeping those two lists synchronized. I
> > think you will need to create a release note document that includes all
> > names, then copy that to a website and remove the names just before the
> > release is packaged.
>
> Unless Bruce is doing more work than I think he is, the attribution data
> going into the release notes is just coming from whatever the committer
> said in the commit log message. I believe that we've generally been

Yes, that's all I do. "Bruce is doing more work than I think he is"
gave me a chuckle. ;-)

> careful to credit the patch author, but I'm less confident that everyone
> who merited a "review credit" always gets mentioned --- that would
> require going through the entire review thread at commit time, and I for
> one can't say that I do that.
>
> If we're going to try harder to ensure that reviewers are credited,
> it'd probably be better to take both the commit log and the release
> notes out of that loop.

I assume you are suggesting we _not_ use the commit log and release
notes for reviewer credit. Good point; we might be able to pull that
from the commit-fest app, though you then need to match that with the
release note text.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

Andrew Dunstan escribió:
>
> On 06/27/2013 02:38 PM, Josh Berkus wrote:
> >>If we're going to try harder to ensure that reviewers are credited,
> >>it'd probably be better to take both the commit log and the release
> >>notes out of that loop.
> >I'd pull the reviewers out of the CF app. Even in its current
> >implementation, that's the easiest route.
>
> I have not honestly found these to be terribly accurate.

Yeah, they're not. I do take care to review past threads when crediting
reviewers in commit messages, and I don't even bother to look at the CF
app.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

-----BEGIN PGP SIGNED MESSAGE-----
Hash: RIPEMD160

Josh Berkus wrote:

> I wasn't thinking about doing it every year -- just for 9.3, in order to
> encourage more reviewers, and encourage reviewers to do more reviews.

- -1. It's not cool to set it up and then stop it the next go round.

You want more reviewers? Start by streamlining the process as much as
possible. I pretended I was new to the project and tried to figure
out how to review something. The homepage has no mention of reviewers,
not even if you drill down on some subpages. A Google search does lead
one to:

http://wiki.postgresql.org/wiki/Reviewing_a_Patch

It has some good "you can do it" wordage. However, there is no clear path
on how to actually start reviewing. There is this paragraph with
two links in it:

"The current commitfest is here[1] and has plenty of room for
you to help. You can sign up to become a Round Robin
Reviewer here[2]. Once you have, write a mail to the list
introducing yourself."

[1] Leads to the commitfest, with a nice summary, but no way for new people
to know what to do.

[2] This link is even worse (http://www.postgresql.org/list/pgsql-rrreviewers/)
It's an archive list for pgsql-rrreviewers, with no way to subscribe
and certainly no indication on it or the previous page that "sign up"
means (one might guess) join the mailing list.

Anyway, just food for thought as far as attracting new people. It should
be much easier and more intuitive. As far as "rewarding" current reviewers,
put the names in the release notes, after each item. Full stop.

- --
Greg Sabino Mullane greg(at)turnstep(dot)com
End Point Corporation http://www.endpoint.com/
PGP Key: 0x14964AC8 201306271636
http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8
-----BEGIN PGP SIGNATURE-----

iEYEAREDAAYFAlHMoqIACgkQvJuQZxSWSsgCPACgovKYtxJV59Xro0MlxPDEHIy6
pmAAoOLOAlpO/dPlJbyHypdcY4ZxLCit
=RwMh
-----END PGP SIGNATURE-----

Folks,

Well, I didn't get much in the way of "poll" responses for the straw
poll. However, let me sum up:

-- two hackers thought that reviewers didn't deserve any credit at all.

-- of the majority of respondants, things were about evenly split
between people who favored "big list at the end" and people who favored
"reviewer next to feature". Notably, those who favored "reviewer next
to feature" also thought that our standards for what constitutes a
"review" should be more stringent.

-- reviewers, in general, were unanimous that the only thing which
mattered in terms of "rewarding" reviewers was credit in the release
notes, and that other "rewards" were nice but inconsequential.

-- a couple of compromise proposals were made:

a) that reviewers who do actual code modification of the patch get
credited on the feature, and those who just review it get credited at
the bottom of the release notes, or

b) that all "names" move to a web page on www.postgresql.org and come
out of the release notes entirely.

Speaking as a commitfest manager, I favor compromise proposal (a),
personally. Does (a) seem somehow terrible to anyone?

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

Josh Berkus wrote:

> -- a couple of compromise proposals were made:
>
> a) that reviewers who do actual code modification of the patch get
> credited on the feature, and those who just review it get credited at
> the bottom of the release notes, or
>
> b) that all "names" move to a web page on www.postgresql.org and come
> out of the release notes entirely.
>
> Speaking as a commitfest manager, I favor compromise proposal (a),
> personally. Does (a) seem somehow terrible to anyone?

I like (a) myself, though I'd amend it so that "extensive work on the
patch" is required to qualify as co-author, which may or may not be
code.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 07/12/2013 01:28 PM, Alvaro Herrera wrote:
> Josh Berkus wrote:
>
>> -- a couple of compromise proposals were made:
>>
>> a) that reviewers who do actual code modification of the patch get
>> credited on the feature, and those who just review it get credited at
>> the bottom of the release notes, or
>>
>> b) that all "names" move to a web page on www.postgresql.org and come
>> out of the release notes entirely.
>>
>> Speaking as a commitfest manager, I favor compromise proposal (a),
>> personally. Does (a) seem somehow terrible to anyone?
> I like (a) myself, though I'd amend it so that "extensive work on the
> patch" is required to qualify as co-author, which may or may not be
> code.
>

I'd probably say "substantial" or "non-trivial", but otherwise +1

cheers

andrew

On 07/12/2013 10:49 AM, Andrew Dunstan wrote:
>
>
> On 07/12/2013 01:28 PM, Alvaro Herrera wrote:
>> Josh Berkus wrote:
>>
>>> -- a couple of compromise proposals were made:
>>>
>>> a) that reviewers who do actual code modification of the patch get
>>> credited on the feature, and those who just review it get credited at
>>> the bottom of the release notes, or
>>>

>
> I'd probably say "substantial" or "non-trivial", but otherwise +1

Right cause if a reviewer ends up writing (or cleaning up) all the docs,
I would say they deserve very close to equal credit. As an example.

JD

--
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 Fri, Jul 12, 2013 at 12:18:15PM -0700, Joshua D. Drake wrote:
>
> On 07/12/2013 10:49 AM, Andrew Dunstan wrote:
> >
> >
> >On 07/12/2013 01:28 PM, Alvaro Herrera wrote:
> >>Josh Berkus wrote:
> >>
> >>>-- a couple of compromise proposals were made:
> >>>
> >>>a) that reviewers who do actual code modification of the patch get
> >>>credited on the feature, and those who just review it get credited at
> >>>the bottom of the release notes, or
> >>>
>
> >
> >I'd probably say "substantial" or "non-trivial", but otherwise +1
>
> Right cause if a reviewer ends up writing (or cleaning up) all the
> docs, I would say they deserve very close to equal credit. As an
> example.

I can do whatever we agree to in the release notes. The big question
is whether committers can properly document these people. I do think
the names are going to overwhelm the release note items and we will
_again_ remove some or all names.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

Bruce Momjian wrote:
> On Fri, Jul 12, 2013 at 12:18:15PM -0700, Joshua D. Drake wrote:

> > Right cause if a reviewer ends up writing (or cleaning up) all the
> > docs, I would say they deserve very close to equal credit. As an
> > example.
>
> I can do whatever we agree to in the release notes. The big question
> is whether committers can properly document these people.

I don't see why not. Most of them, if not all, already do.

> I do think the names are going to overwhelm the release note items and
> we will _again_ remove some or all names.

There's plenty of opinion to the contrary; but then it's just opinion.
I think the idea of trying it at least once has merit.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Fri, Aug 2, 2013 at 04:43:30PM -0400, Alvaro Herrera wrote:
> Bruce Momjian wrote:
> > On Fri, Jul 12, 2013 at 12:18:15PM -0700, Joshua D. Drake wrote:
>
> > > Right cause if a reviewer ends up writing (or cleaning up) all the
> > > docs, I would say they deserve very close to equal credit. As an
> > > example.
> >
> > I can do whatever we agree to in the release notes. The big question
> > is whether committers can properly document these people.
>
> I don't see why not. Most of them, if not all, already do.

Do they record which reviewers changed code and which just gave
feedback?

> > I do think the names are going to overwhelm the release note items and
> > we will _again_ remove some or all names.
>
> There's plenty of opinion to the contrary; but then it's just opinion.
> I think the idea of trying it at least once has merit.

This is what the 9.2 release notes looked like before I remove the
reviewers:

http://momjian.us/expire/release-9-2.html

Most items had 2-3 names, and it was widely rejected. Of course, these
were all reviewers, not just those that changed the code. I did not
have details of which reviewers changed code and which just gave
feedback.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

On 08/02/2013 01:56 PM, Bruce Momjian wrote:
> On Fri, Aug 2, 2013 at 04:43:30PM -0400, Alvaro Herrera wrote:
>> Bruce Momjian wrote:
>>> On Fri, Jul 12, 2013 at 12:18:15PM -0700, Joshua D. Drake wrote:
>>
>>>> Right cause if a reviewer ends up writing (or cleaning up) all the
>>>> docs, I would say they deserve very close to equal credit. As an
>>>> example.
>>>
>>> I can do whatever we agree to in the release notes. The big question
>>> is whether committers can properly document these people.
>>
>> I don't see why not. Most of them, if not all, already do.

It is also my thinking that it is the job of the CommitFestManager to
re-enforce this list by looking through the review list. If we do this
on a per-CF basis, the workload won't become substantial; it's only if
we wait until beta that it gets overwhelming.

The CFM needs to supply the list of "reviewers at the end" anyway.

> Most items had 2-3 names, and it was widely rejected. Of course, these
> were all reviewers, not just those that changed the code. I did not
> have details of which reviewers changed code and which just gave
> feedback.

I think "widely rejected" is an exaggeration; a few people objected
stenuously. And the primary objection voiced was that people who did
"it compiles!" shouldn't get equal credit with the original author of
the patch. Which we're not proposing to do.

BTW, all of this I'm talking about the 9.4 release notes, where we have
the opportunity to start from the first CF. There's the question of what
to do about the *9.3* release notes, which I'll address in a seperate email.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

Bruce, all:

Per previous email, I wanted to make a specific proposal for what to do
on the 9.3 release notes. This is because, without policy set, we have
not been tracking which reviewers make "substantial changes" in 9.3, and
listing some-but-not-all of them would cause a lot of unhappiness among
the omitted reviewers.

Therefore, I propose for 9.3 only that we just have the list at the end
of the release notes, including all reviewers. That way we can make
sure to include everyone equally.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On Fri, Aug 2, 2013 at 02:10:17PM -0700, Josh Berkus wrote:
> Bruce, all:
>
> Per previous email, I wanted to make a specific proposal for what to do
> on the 9.3 release notes. This is because, without policy set, we have
> not been tracking which reviewers make "substantial changes" in 9.3, and
> listing some-but-not-all of them would cause a lot of unhappiness among
> the omitted reviewers.
>
> Therefore, I propose for 9.3 only that we just have the list at the end
> of the release notes, including all reviewers. That way we can make
> sure to include everyone equally.

Yes, there is no way to add people to the 9.3 release note items at this
point --- my assumption is this is all 9.4 discussion, or maybe 9.5 as
we have already committed quite a bit to 9.4.

I don't think dumping reviewer names at the bottom of the 9.3 release
notes is what the majority want, and it seems like an ugly short-term
solution.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

On Fri, Aug 2, 2013 at 02:07:53PM -0700, Josh Berkus wrote:
> On 08/02/2013 01:56 PM, Bruce Momjian wrote:
> > On Fri, Aug 2, 2013 at 04:43:30PM -0400, Alvaro Herrera wrote:
> >> Bruce Momjian wrote:
> >>> On Fri, Jul 12, 2013 at 12:18:15PM -0700, Joshua D. Drake wrote:
> >>
> >>>> Right cause if a reviewer ends up writing (or cleaning up) all the
> >>>> docs, I would say they deserve very close to equal credit. As an
> >>>> example.
> >>>
> >>> I can do whatever we agree to in the release notes. The big question
> >>> is whether committers can properly document these people.
> >>
> >> I don't see why not. Most of them, if not all, already do.
>
> It is also my thinking that it is the job of the CommitFestManager to
> re-enforce this list by looking through the review list. If we do this
> on a per-CF basis, the workload won't become substantial; it's only if
> we wait until beta that it gets overwhelming.

Based on existing workflow, we need those reviewer names in the commit
message. I don't see how the CommitFestManager can help with that.

> The CFM needs to supply the list of "reviewers at the end" anyway.

Why?

> > Most items had 2-3 names, and it was widely rejected. Of course, these
> > were all reviewers, not just those that changed the code. I did not
> > have details of which reviewers changed code and which just gave
> > feedback.
>
> I think "widely rejected" is an exaggeration; a few people objected
> stenuously. And the primary objection voiced was that people who did
> "it compiles!" shouldn't get equal credit with the original author of
> the patch. Which we're not proposing to do.

Well, I had to remove it pretty quickly, so that is my recolletion.

> BTW, all of this I'm talking about the 9.4 release notes, where we have
> the opportunity to start from the first CF. There's the question of what
> to do about the *9.3* release notes, which I'll address in a seperate email.

I am worried we are talking about 9.5 as we have already committed quite
a bit to 9.4.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

On 08/02/2013 02:24 PM, Bruce Momjian wrote:

> Based on existing workflow, we need those reviewer names in the commit
> message. I don't see how the CommitFestManager can help with that.

We can change the workflow. It's ours, there's no government agency
mandating it.

Anyway, the list from the CFM would just be to make sure nobody got
missed; it's a double-check on the commit messages.

>> The CFM needs to supply the list of "reviewers at the end" anyway.
>
> Why?

Who else would do it?

>> BTW, all of this I'm talking about the 9.4 release notes, where we have
>> the opportunity to start from the first CF. There's the question of what
>> to do about the *9.3* release notes, which I'll address in a seperate email.
>
> I am worried we are talking about 9.5 as we have already committed quite
> a bit to 9.4.

You're making a big deal out of what's a minor clerical detail. Don't
let minutia which any secretary could take care of get in the way of an
important project goal, that is, rewarding reviewers so that lack of
reviewers stops being a major project bottleneck.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On 08/02/2013 02:23 PM, Bruce Momjian wrote:

> I don't think dumping reviewer names at the bottom of the 9.3 release
> notes is what the majority want, and it seems like an ugly short-term
> solution.

It's better than not crediting the reviewers *at all*, which is the only
alternative I can think of.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On Fri, Aug 2, 2013 at 02:36:42PM -0700, Josh Berkus wrote:
> On 08/02/2013 02:24 PM, Bruce Momjian wrote:
>
> > Based on existing workflow, we need those reviewer names in the commit
> > message. I don't see how the CommitFestManager can help with that.
>
> We can change the workflow. It's ours, there's no government agency
> mandating it.
>
> Anyway, the list from the CFM would just be to make sure nobody got
> missed; it's a double-check on the commit messages.
>
> >> The CFM needs to supply the list of "reviewers at the end" anyway.
> >
> > Why?
>
> Who else would do it?
>
> >> BTW, all of this I'm talking about the 9.4 release notes, where we have
> >> the opportunity to start from the first CF. There's the question of what
> >> to do about the *9.3* release notes, which I'll address in a seperate email.
> >
> > I am worried we are talking about 9.5 as we have already committed quite
> > a bit to 9.4.
>
> You're making a big deal out of what's a minor clerical detail. Don't
> let minutia which any secretary could take care of get in the way of an
> important project goal, that is, rewarding reviewers so that lack of
> reviewers stops being a major project bottleneck.

You are approaching this like it is a done deal and everyone agrees to
it.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

On 08/02/2013 03:18 PM, Bruce Momjian wrote:
>> You're making a big deal out of what's a minor clerical detail. Don't
>> let minutia which any secretary could take care of get in the way of an
>> important project goal, that is, rewarding reviewers so that lack of
>> reviewers stops being a major project bottleneck.
>
> You are approaching this like it is a done deal and everyone agrees to
> it.

We already discussed it in the thread ad nauseum, and arrived at a
compromise which everyone could live with. So from that perspective, it
*is* a done deal, at least as far as 9.4 is concerned. At some point,
we need to make a decision and move forward, instead of rehashing the
same arguments forever.

So if you're raising an objection to the compromise which many people
already agreed to, then raise an objection and back it up. But don't
sandbag.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On Fri, Aug 2, 2013 at 03:55:27PM -0700, Josh Berkus wrote:
> On 08/02/2013 03:18 PM, Bruce Momjian wrote:
> >> You're making a big deal out of what's a minor clerical detail. Don't
> >> let minutia which any secretary could take care of get in the way of an
> >> important project goal, that is, rewarding reviewers so that lack of
> >> reviewers stops being a major project bottleneck.
> >
> > You are approaching this like it is a done deal and everyone agrees to
> > it.
>
> We already discussed it in the thread ad nauseum, and arrived at a
> compromise which everyone could live with. So from that perspective, it
> *is* a done deal, at least as far as 9.4 is concerned. At some point,
> we need to make a decision and move forward, instead of rehashing the
> same arguments forever.
>
> So if you're raising an objection to the compromise which many people
> already agreed to, then raise an objection and back it up. But don't
> sandbag.

There are three issues here:

1. What will best motive reviewers?
2. What is a reasonable effort to accomplish #1?
3. What is acceptable for release note readers?

You seem to be only focused on #1, and you don't want to address the
other items --- that's fine --- I will still be around if people lose
interest or the system becomes unworkable.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

Bruce,

> There are three issues here:
>
> 1. What will best motive reviewers?
> 2. What is a reasonable effort to accomplish #1?
> 3. What is acceptable for release note readers?
>
> You seem to be only focused on #1, and you don't want to address the
> other items --- that's fine --- I will still be around if people lose
> interest or the system becomes unworkable.

Both I and other people have already addressed #2 and #3. You're also
having a huge failure of perspective here. Motivating reviewers allows
our project to continue developing PostgreSQL. Items 2 and 3 are so
insignificant in comparison to that as to not be worth discussing.

In the novels The Hitchhiker's Guide To The Galaxy, there's a spaceship
which has been waiting 1000 years to take off because it's waiting for a
load of lemon-soaked paper napkins to be loaded. You are being Mr.
Lemon-Soaked Paper Napkin.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On 2013-08-07 10:04:08 -0700, Josh Berkus wrote:
> Bruce,
>
> > There are three issues here:
> >
> > 1. What will best motive reviewers?
> > 2. What is a reasonable effort to accomplish #1?
> > 3. What is acceptable for release note readers?
> >
> > You seem to be only focused on #1, and you don't want to address the
> > other items --- that's fine --- I will still be around if people lose
> > interest or the system becomes unworkable.
>
> Both I and other people have already addressed #2 and #3. You're also
> having a huge failure of perspective here. Motivating reviewers allows
> our project to continue developing PostgreSQL. Items 2 and 3 are so
> insignificant in comparison to that as to not be worth discussing.
>
> In the novels The Hitchhiker's Guide To The Galaxy, there's a spaceship
> which has been waiting 1000 years to take off because it's waiting for a
> load of lemon-soaked paper napkins to be loaded. You are being Mr.
> Lemon-Soaked Paper Napkin.

Holy crap Josh. I agree that we should give reviews space in the release
notes, but could you please cut the ad-hominem bullshit? You're
preventing your own success here.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Wed, Aug 7, 2013 at 10:04:08AM -0700, Josh Berkus wrote:
> Bruce,
>
> > There are three issues here:
> >
> > 1. What will best motive reviewers?
> > 2. What is a reasonable effort to accomplish #1?
> > 3. What is acceptable for release note readers?
> >
> > You seem to be only focused on #1, and you don't want to address the
> > other items --- that's fine --- I will still be around if people lose
> > interest or the system becomes unworkable.
>
> Both I and other people have already addressed #2 and #3. You're also
> having a huge failure of perspective here. Motivating reviewers allows
> our project to continue developing PostgreSQL. Items 2 and 3 are so
> insignificant in comparison to that as to not be worth discussing.

OK, my analysis was accurate then --- for you, #1 overshadows numbers 2
and 3. I don't share those priorities, and the work required is
unbounded (no #2), so I will not perform additional work to help with
#1.

> In the novels The Hitchhiker's Guide To The Galaxy, there's a spaceship
> which has been waiting 1000 years to take off because it's waiting for a
> load of lemon-soaked paper napkins to be loaded. You are being Mr.
> Lemon-Soaked Paper Napkin.

Actually, for me, motiving reviewers seems like the Lemon-Soaked Paper
Napkins, as it requires unbounded effort and its importance is not being
balanced with other priorities.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

On 08/07/2013 10:10 AM, Andres Freund wrote:
> On 2013-08-07 10:04:08 -0700, Josh Berkus wrote:
>> In the novels The Hitchhiker's Guide To The Galaxy, there's a spaceship
>> which has been waiting 1000 years to take off because it's waiting for a
>> load of lemon-soaked paper napkins to be loaded. You are being Mr.
>> Lemon-Soaked Paper Napkin.
>
> Holy crap Josh. I agree that we should give reviews space in the release
> notes, but could you please cut the ad-hominem bullshit? You're
> preventing your own success here.

Huh? I was comparing Bruce's *argument* to a scene in "Hitchhiker's
Guide". How is that ad hominem?

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

Bruce,

> Actually, for me, motiving reviewers seems like the Lemon-Soaked Paper
> Napkins, as it requires unbounded effort and its importance is not being
> balanced with other priorities.

Let me be absolutely clear here: You do not think that the work
reviewers do is important at all, and you think that our project has
more than enough reviewers? I want to be crystal-clear on your opinion.

I will point out that you did exactly zero reviews in the last
commitfest, which makes me wonder what your opinion of "we have enough
reviewers" is based on.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

Josh,

* Josh Berkus (josh(at)agliodbs(dot)com) wrote:
> > Actually, for me, motiving reviewers seems like the Lemon-Soaked Paper
> > Napkins, as it requires unbounded effort and its importance is not being
> > balanced with other priorities.
>
> Let me be absolutely clear here: You do not think that the work
> reviewers do is important at all, and you think that our project has
> more than enough reviewers? I want to be crystal-clear on your opinion.

Bruce certainly didn't say that and it's rather disingenuous to claim
that he did. What I read is that he simply pointed out that we have
multiple priorities and need to consider work on acquiring new
reviewers in balance with the rest.

Also mentioned is that it's unclear how one might bound the work of
getting new reviewers- you can't say "it'll take X hours to get enough
reviewers" or even "it'll take X hours to get 5 new reviewers".

Thanks,

Stephen

On Wed, Aug 7, 2013 at 12:07:32PM -0700, Josh Berkus wrote:
> Bruce,
>
> > Actually, for me, motiving reviewers seems like the Lemon-Soaked Paper
> > Napkins, as it requires unbounded effort and its importance is not being
> > balanced with other priorities.
>
> Let me be absolutely clear here: You do not think that the work
> reviewers do is important at all, and you think that our project has
> more than enough reviewers? I want to be crystal-clear on your opinion.
>
> I will point out that you did exactly zero reviews in the last
> commitfest, which makes me wonder what your opinion of "we have enough
> reviewers" is based on.

Only Lemon-Soaked Paper Napkins users think that everything is binary
--- every effort has to be balanced against the work involved, which was
#2 on my list.

You are getting into some kind of loop where not wanting to expend
unlimited effort on something means, to you, that the person doesn't
think the goal is important. Effort has to be balanced. This is not
the first time I have seen such loops. And why do you even care about
my opinion?

And I have never said "we have enough reviewers". Is this where you say
I should be doing more? I thought we dealt with that already.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

Bruce,

> You are getting into some kind of loop where not wanting to expend
> unlimited effort on something means, to you, that the person doesn't
> think the goal is important. Effort has to be balanced. This is not
> the first time I have seen such loops. And why do you even care about
> my opinion?

Aha, OK. So you're talking about all the different things we might do
to get more reviewers. I'm only talking about adding reviewers to the
bottom of the release notes, which is certainly a bounded activity of
*very* limited effort. Which is why I was confused and aghast at your
talk of "unbounded work".

To be completely clear: I am talking only about the compromise discussed
on this thread, namely:

a) listing reviewers who did "extensive work" as co-authors on the
patch, and
b) listing other reviewers at the bottom of the release notes.

Per earlier discussions. You started this thread by claiming that
adding the reviewers to 9.4 would be too hard, and I argued that it
would not and in fact I'm already working on it. Nothing I've talked
about in this thread has been about anything else.
--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On 08/07/2013 10:35 AM, Bruce Momjian wrote:

> Actually, for me, motiving reviewers seems like the Lemon-Soaked Paper
> Napkins, as it requires unbounded effort and its importance is not being
> balanced with other priorities.
>

Ignoring the non-productive part of this thread, I would like to mention
that motivating reviewers is not necessarily complicated. We just have
to ask ourselves what motivates a person:

The feeling that their work is worthwhile, productive, will be
appreciated and that they will receive recognition for the effort.

Right now, we do not publicly outside of the dome that is -hackers
provide those incentives. Give reviewers the just recognition they
deserve and I believe we will see more reviewing effort.

Sincerely,

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 Wed, Aug 7, 2013 at 12:39:01PM -0700, Josh Berkus wrote:
> Bruce,
>
> > You are getting into some kind of loop where not wanting to expend
> > unlimited effort on something means, to you, that the person doesn't
> > think the goal is important. Effort has to be balanced. This is not
> > the first time I have seen such loops. And why do you even care about
> > my opinion?
>
> Aha, OK. So you're talking about all the different things we might do
> to get more reviewers. I'm only talking about adding reviewers to the
> bottom of the release notes, which is certainly a bounded activity of
> *very* limited effort. Which is why I was confused and aghast at your
> talk of "unbounded work".

Well, reviewers on the bottom was just for 9.3 or 9.4, but the final
goal was to get reviewers who modified patches credited with the release
note items. I asked how that was to be accomplished, and suggested that
the only practical way would be for every committer to check the patch
chain to see who else had modified the patch.

You suggested something about the commit-fest-manager doing it, and I
couldn't see how that would help because it has to be in the commit
message at the time the release notes are being written. You said our
release note writing process was not written stone, and that we had to
do whatever it takes to get those names on the items in the release
notes. At that point I pointed out that there was no consideration of
the effort necessary to accomplish this, and that's how we got here
today.

> To be completely clear: I am talking only about the compromise discussed
> on this thread, namely:
>
> a) listing reviewers who did "extensive work" as co-authors on the
> patch, and

See above --- I need to know how that is going to get to the release
note items _with_ reasonable effort.

> b) listing other reviewers at the bottom of the release notes.

Yes, that is somewhat easy in that we can get the names from the
commit-fest app, but it doesn't include reviewers who replied via email
but did not record their names on the commit-fest app. I can tell you
from my release note writing experience that a partial job in this area
is likely to get lots of negative feedback from people who are
excluded.

As an example, I got a pg_upgrade patch fix for 9.2, thought it was
ugly, tried other methods, ended up doing the same thing the original
patch author did, but didn't mention the patch author because I had
forgotten I ended up with the same fix. When the minor release notes
came out, the person complained on the hackers list, and Simon and Tom
had to apologize as I was away:

http://www.postgresql.org/message-id/CABRT9RAe3zxdins=BBysjqEPA8=NqnxWtA4A0XM01PbdVbmC_A@mail.gmail.com

My point is this has to be done accurately.

> Per earlier discussions. You started this thread by claiming that
> adding the reviewers to 9.4 would be too hard, and I argued that it
> would not and in fact I'm already working on it. Nothing I've talked
> about in this thread has been about anything else.

You have to distinguish between names at the end of the release notes,
and names on release note items, and you have to tell us how this going
to happen with reasonable effort.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

> Well, reviewers on the bottom was just for 9.3 or 9.4, but the final
> goal was to get reviewers who modified patches credited with the release
> note items. I asked how that was to be accomplished, and suggested that
> the only practical way would be for every committer to check the patch
> chain to see who else had modified the patch.

Actually, it's not that hard. Someone who modified the patch is going
to post it to -hackers, and we have that all in the archives. Michael
and I are combing through the threads from CF1 now to get that list;
you're talking a few hours effort at most.

Of course, the actual patch author/committers need to verify that these
people actually did a lot of *useful* work, but that isn't much effort
from them.

> You suggested something about the commit-fest-manager doing it, and I
> couldn't see how that would help because it has to be in the commit
> message at the time the release notes are being written.

Why? You didn't provide *any* justification as to why the release notes
could not include input *in addition to* commit messages. In fact, the
release notes *do* incorporate additional input, every year.

> You said our
> release note writing process was not written stone, and that we had to
> do whatever it takes to get those names on the items in the release
> notes. At that point I pointed out that there was no consideration of
> the effort necessary to accomplish this, and that's how we got here
> today.

The only additional effort I'm asking of you, Bruce, is to accept
patches on the release notes. That really doesn't seem like an
unreasonable request.

> Yes, that is somewhat easy in that we can get the names from the
> commit-fest app, but it doesn't include reviewers who replied via email
> but did not record their names on the commit-fest app. I can tell you
> from my release note writing experience that a partial job in this area
> is likely to get lots of negative feedback from people who are
> excluded.

That's why it'll be a social process. Next week I'll be posting a list
of patches and reviewers from CF1 to this list for other people to
correct and expand. Just like the code itself, if everybody reviews the
list of reviewers, it'll be as good as we can get it.

> My point is this has to be done accurately.

It will be just as accurate as the current process, which, as you've
just pointed out, is not 100% accurate either.

> You have to distinguish between names at the end of the release notes,
> and names on release note items, and you have to tell us how this going
> to happen with reasonable effort.

You're making a mountain out of a molehill here. I've already spent
more time arguing with you than it will take me to do the work.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On Wed, Aug 7, 2013 at 01:48:06PM -0700, Josh Berkus wrote:
>
> > Well, reviewers on the bottom was just for 9.3 or 9.4, but the final
> > goal was to get reviewers who modified patches credited with the release
> > note items. I asked how that was to be accomplished, and suggested that
> > the only practical way would be for every committer to check the patch
> > chain to see who else had modified the patch.
>
> Actually, it's not that hard. Someone who modified the patch is going
> to post it to -hackers, and we have that all in the archives. Michael
> and I are combing through the threads from CF1 now to get that list;
> you're talking a few hours effort at most.

Michael who?

9.4 CF1? Where are you recording the names? In the commitfest app?

> Of course, the actual patch author/committers need to verify that these
> people actually did a lot of *useful* work, but that isn't much effort
> from them.

OK, so the process is independent of commit activity. You realize that
if someone significantly modifies a patch we already have them in the
commit message as an author and on the release note item, right? So you
are really looking for reviews that modify the patch but not enough for
a committer to include their name in the commit message as an author.

> > You suggested something about the commit-fest-manager doing it, and I
> > couldn't see how that would help because it has to be in the commit
> > message at the time the release notes are being written.
>
> Why? You didn't provide *any* justification as to why the release notes
> could not include input *in addition to* commit messages. In fact, the
> release notes *do* incorporate additional input, every year.

Yes, we can always add --- the problem is getting the content to add.

> > You said our
> > release note writing process was not written stone, and that we had to
> > do whatever it takes to get those names on the items in the release
> > notes. At that point I pointed out that there was no consideration of
> > the effort necessary to accomplish this, and that's how we got here
> > today.
>
> The only additional effort I'm asking of you, Bruce, is to accept
> patches on the release notes. That really doesn't seem like an
> unreasonable request.

Anyone can commit patches to the release notes. I am unlikely to do it,
as I lack confidence in the process, for reasons already outlined.

> > Yes, that is somewhat easy in that we can get the names from the
> > commit-fest app, but it doesn't include reviewers who replied via email
> > but did not record their names on the commit-fest app. I can tell you
> > from my release note writing experience that a partial job in this area
> > is likely to get lots of negative feedback from people who are
> > excluded.
>
> That's why it'll be a social process. Next week I'll be posting a list
> of patches and reviewers from CF1 to this list for other people to
> correct and expand. Just like the code itself, if everybody reviews the
> list of reviewers, it'll be as good as we can get it.

OK, at least that is a plan. Is that your plan for the future too?

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

> Michael who?

Blackwell, asssistant CFM for this CF.

> 9.4 CF1? Where are you recording the names? In the commitfest app?

Right now in a googledoc. The CF app has no such capability now,
although Magnus' new app might in the future.

> OK, so the process is independent of commit activity. You realize that
> if someone significantly modifies a patch we already have them in the
> commit message as an author and on the release note item, right? So you
> are really looking for reviews that modify the patch but not enough for
> a committer to include their name in the commit message as an author.

Oh, good point, I can look at the commit messages for where I don't need
to bother. However, you pointed out that *during* CF1, the committers
*didn't know* that they were supposed to include reviewers who did
"major work" in the commit message. And they might miss them in the future.

> Anyone can commit patches to the release notes. I am unlikely to do it,
> as I lack confidence in the process, for reasons already outlined.

Bruce, you are steadfastly resistant to change of any kind.

>> That's why it'll be a social process. Next week I'll be posting a list
>> of patches and reviewers from CF1 to this list for other people to
>> correct and expand. Just like the code itself, if everybody reviews the
>> list of reviewers, it'll be as good as we can get it.
>
> OK, at least that is a plan. Is that your plan for the future too?

Sure. It's the open source way.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

On Wed, Aug 7, 2013 at 04:39:49PM -0700, Josh Berkus wrote:
> > OK, so the process is independent of commit activity. You realize that
> > if someone significantly modifies a patch we already have them in the
> > commit message as an author and on the release note item, right? So you
> > are really looking for reviews that modify the patch but not enough for
> > a committer to include their name in the commit message as an author.
>
> Oh, good point, I can look at the commit messages for where I don't need
> to bother. However, you pointed out that *during* CF1, the committers
> *didn't know* that they were supposed to include reviewers who did
> "major work" in the commit message. And they might miss them in the future.

Well, the clear way to do this would be to ask the committers if they
can reliably take on this job. You are right for CF1 they treated
reviewer patch modifications just like anyone else, but of course, the
larger question is whether you _should_ treat the reviewers different,
because other people are reviewers just not recorded as CF reviewers.
Another question is whether committers are going to recognize CF
reviewers vs. ordinary patch modifiers.

> > Anyone can commit patches to the release notes. I am unlikely to do it,
> > as I lack confidence in the process, for reasons already outlined.
>
> Bruce, you are steadfastly resistant to change of any kind.

Perhaps I am pessimistic, but I need to have confidence in the process,
and at this point, I don't, and considering how long it took for me to
get an explanation of the process, this seems prudent.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +