Re: [9.4 CF 1] The Commitfest Slacker List

Folks,

For 9.2, we adopted it as policy that anyone submitting a patch to a
commitfest is expected to review at least one patch submitted by someone
else. And that failure to do so would affect the attention your patches
received in the future. For that reason, I'm publishing the list below
of people who submitted patches and have not yet claimed any patch in
the commitfest to review.

For those of you who are contributing patches for your company, please
let your boss know that reviewing is part of contributing, and that if
you don't do the one you may not be able to do the other.

The two lists below, idle submitters and committers, constitutes 26
people. This *outnumbers* the list of contributors who are busy
reviewing patches -- some of them four or more patches. If each of
these people took just *one* patch to review, it would almost entirely
clear the list of patches which do not have a reviewer. If these folks
continue not to do reviews, this commitfest will drag on for at least 2
weeks past its closure date.

Andrew Geirth
Nick White
Peter Eisentrout
Alexander Korotkov
Etsuro Fujita
Hari Babu
Jameison Martin
Jon Nelson
Oleg Bartunov
Chris Farmiloe
Samrat Revagade
Alexander Lakhin
Mark Kirkwood
Liming Hu
Maciej Gajewski
Josh Kuperschmidt
Mark Wong
Gurjeet Singh
Robins Tharakan
Tatsuo Ishii
Karl O Pinc

Additionally, the following committers are not listed as reviewers on
any patch. Note that I have no way to search which ones might be
*committers* on a patch, so these folks are not necessarily slackers
(although with Bruce, we know for sure):

Bruce Momjian (momjian)
Michael Meskes (meskes)
Teodor Sigaev (teodor)
Andrew Dunstan (adunstan)
Magnus Hagander (mha)
Itagaki Takahiro (itagaki)

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

On 06/24/2013 12:41 AM, Josh Berkus wrote:
> Folks,
>
> For 9.2, we adopted it as policy that anyone submitting a patch to a
> commitfest is expected to review at least one patch submitted by someone
> else. And that failure to do so would affect the attention your patches
> received in the future. For that reason, I'm publishing the list below
> of people who submitted patches and have not yet claimed any patch in
> the commitfest to review.
>
> For those of you who are contributing patches for your company, please
> let your boss know that reviewing is part of contributing, and that if
> you don't do the one you may not be able to do the other.
>
> The two lists below, idle submitters and committers, constitutes 26
> people. This *outnumbers* the list of contributors who are busy
> reviewing patches -- some of them four or more patches. If each of
> these people took just *one* patch to review, it would almost entirely
> clear the list of patches which do not have a reviewer. If these folks
> continue not to do reviews, this commitfest will drag on for at least 2
> weeks past its closure date.
>
> Andrew Geirth
> Nick White
> Peter Eisentrout
> Alexander Korotkov
> Etsuro Fujita
> Hari Babu
> Jameison Martin
> Jon Nelson
> Oleg Bartunov
> Chris Farmiloe
> Samrat Revagade
> Alexander Lakhin
> Mark Kirkwood
> Liming Hu
> Maciej Gajewski
> Josh Kuperschmidt
> Mark Wong
> Gurjeet Singh
> Robins Tharakan
> Tatsuo Ishii
> Karl O Pinc
>
> Additionally, the following committers are not listed as reviewers on
> any patch. Note that I have no way to search which ones might be
> *committers* on a patch, so these folks are not necessarily slackers
> (although with Bruce, we know for sure):
>
> Bruce Momjian (momjian)
> Michael Meskes (meskes)
> Teodor Sigaev (teodor)
> Andrew Dunstan (adunstan)
> Magnus Hagander (mha)
> Itagaki Takahiro (itagaki)
>

I think we maybe need to be a bit more careful about a name and shame
policy, or it will be ignored. I actually reviewed Peter's emacs config
patch just yesterday, although I didn't note that on the Commitfest app.
(Incidentally, based on Tom's later comments I think we should probably
mark that one as rejected.)

Anyway, I have claimed the VPATH patches for review, and will look at
them today.

cheers

andrew

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> I think we maybe need to be a bit more careful about a name and shame
> policy, or it will be ignored.

I very much don't like that idea of publishing a list of names either.
Editing the reviewer field and sending personal notices is fine by me,
but name and shame is walking the line…

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

Maybe this policy should be mentioned on the Wiki, so newbies like myself
(who wouldn't even dare reviewing patches submitted be seasoned hackers)
are not surprised by seeing own name on a shame wall?

M

On 06/24/2013 05:40 PM, Maciej Gajewski wrote:
> Maybe this policy should be mentioned on the Wiki, so newbies like
> myself (who wouldn't even dare reviewing patches submitted be seasoned
> hackers) are not surprised by seeing own name on a shame wall?

I personally would prefer if the email was sent to those who should be
reminded instead to the list. My personal experience is that personal
reminders are more effective than public naming and shaming.

--
Andreas Karlsson

On 06/24/2013 08:40 AM, Maciej Gajewski wrote:
> Maybe this policy should be mentioned on the Wiki, so newbies like
> myself (who wouldn't even dare reviewing patches submitted be seasoned
> hackers) are not surprised by seeing own name on a shame wall?

It is mentioned. Of course now I can't find it but it is there.

However, I believe you are taking the wrong perspective on this. This is
not a shame wall. It is a transparent reminder of the policy and those
who have not assisted in reviewing a patch but have submitted a patch
themselves.

In short, leave the ego at the door.

Sincerely,

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

"Joshua D. Drake" <jd(at)commandprompt(dot)com> writes:
> In short, leave the ego at the door.

That's not the problem. Let's welcome those who are able to contribute
their time and skills without making it harder for them. Motivation here
shoulnd't be how to avoid getting enlisted on the shame wall.

My opinion is that if we play the game that way, we will only achieve to
push contributors out of the community and review process. Please kindly
reconsider and don't publish any such backward list again.

Instead, I don't know, fetch some SPI money to offer a special poster or
unique one-time-edition only hoodie or a signed mug or whatever to extra
proficient contributors and turn that into a game people want to win.

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

On 06/24/2013 05:54 PM, Joshua D. Drake wrote:
>
> On 06/24/2013 08:40 AM, Maciej Gajewski wrote:
>> Maybe this policy should be mentioned on the Wiki, so newbies like
>> myself (who wouldn't even dare reviewing patches submitted be seasoned
>> hackers) are not surprised by seeing own name on a shame wall?
>
> It is mentioned. Of course now I can't find it but it is there.
>
> However, I believe you are taking the wrong perspective on this. This
> is not a shame wall. It is a transparent reminder of the policy and
> those who have not assisted in reviewing a patch but have submitted a
> patch themselves.
>
> In short, leave the ego at the door.
Would be much easier to do if you did not post this to the list :)

I guess you can also extract the e-mails and post only to the "slackers".

--
Hannu Krosing
PostgreSQL Consultant
Performance, Scalability and High Availability
2ndQuadrant Nordic OÜ

On 06/24/2013 08:01 AM, Dimitri Fontaine wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>> I think we maybe need to be a bit more careful about a name and shame
>> policy, or it will be ignored.
>
> I very much don't like that idea of publishing a list of names either.
> Editing the reviewer field and sending personal notices is fine by me,
> but name and shame is walking the line…

Actually, every submitter on that list -- including Maciej -- was sent a
personal, private email a week ago. A few (3) chose to take the
opportunity to review things, or promised to do so, including a brand
new Chinese contributor who needed help with English to review his
patch. The vast majority chose not to respond to my email to them at
all. When private email fails, the next step is public email.

Maciej is correct that this policy also belongs on the "how to submit a
patch" wiki page. I will remedy that.

Doing the patch counts yesterday, it became clear to me that the reason
for the patch review pileups was that many people were submitting
patches but not participating in the review process at all. That is, we
have 100 to 150 people a year submitting patches, but relying entirely
on the committers and a few heroic uber-reviewers to do 90% of the patch
review. This is the commitfest problem in a nutshell.

The purpose of the list was to make it completely apparent where the
problem in clearing the patch queue lies, and to get some of our
submitters to do patch review.

Per both of my emails yesterday, I am trying to make sure that this CF
finishes on time. Following the rules passed at the developer meetings
for how CFs are to be run, I am doing so. If the result is
unsatisfactory, I can always resign as CFM.

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

Josh Berkus <josh(at)agliodbs(dot)com> writes:
> patch. The vast majority chose not to respond to my email to them at
> all. When private email fails, the next step is public email.

The only problem I have here is that I don't remember about deciding to
publish a list of failures by public email at all. I hope it's not my
memory failing me here, because then I would have to remember why I
didn't speak up against that idea at the time.

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

On 06/24/2013 10:02 AM, Dimitri Fontaine wrote:
> Josh Berkus <josh(at)agliodbs(dot)com> writes:
>> patch. The vast majority chose not to respond to my email to them at
>> all. When private email fails, the next step is public email.
>
> The only problem I have here is that I don't remember about deciding to
> publish a list of failures by public email at all. I hope it's not my
> memory failing me here, because then I would have to remember why I
> didn't speak up against that idea at the time.

You didn't decide anything. As the CFM, I did. My job for this month
is to make sure that 100% of patches in that queue get reviewed and
either committed or bounced by July 15th. I'm doing my job.

I will be more than happy to resign as CFM and turn it over to someone
else if people have a problem with it.

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

On 2013-06-24 10:10:11 -0700, Josh Berkus wrote:
> On 06/24/2013 10:02 AM, Dimitri Fontaine wrote:
> > Josh Berkus <josh(at)agliodbs(dot)com> writes:
> >> patch. The vast majority chose not to respond to my email to them at
> >> all. When private email fails, the next step is public email.
> >
> > The only problem I have here is that I don't remember about deciding to
> > publish a list of failures by public email at all. I hope it's not my
> > memory failing me here, because then I would have to remember why I
> > didn't speak up against that idea at the time.
>
> You didn't decide anything. As the CFM, I did. My job for this month
> is to make sure that 100% of patches in that queue get reviewed and
> either committed or bounced by July 15th. I'm doing my job.
>
> I will be more than happy to resign as CFM and turn it over to someone
> else if people have a problem with it.

Heck, Josh. People have to be allowed to critize *a small part* of your
work without you understanding it as a fundamental request to step back
from being CFM.

Greetings,

Andres Freund

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

> Instead, I don't know, fetch some SPI money to offer a special poster or
> unique one-time-edition only hoodie or a signed mug or whatever to extra
> proficient contributors and turn that into a game people want to win.

I like that idea too. Provided that we allocate enough funding that I
can have a paid admin handle the shipping etc. Frankly, I'd be up for
the idea that patch reviewers get a special t-shirt for each PostgresQL
release, for reviewing even *one* patch.

Mind you, we wouldn't be able to reward a few reviewers, because they
live in countries to which it's impossible to ship from abroad.

I have previously proposed that all of the reviewers of a given
PostgreSQL release be honored in the release notes as a positive
incentive, and was denied on this from doing so. Not coincidentally, we
don't seem to have any reviewers-at-large anymore.

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

On 06/24/2013 10:10 AM, Josh Berkus wrote:
>
> On 06/24/2013 10:02 AM, Dimitri Fontaine wrote:
>> Josh Berkus <josh(at)agliodbs(dot)com> writes:
>>> patch. The vast majority chose not to respond to my email to them at
>>> all. When private email fails, the next step is public email.
>>
>> The only problem I have here is that I don't remember about deciding to
>> publish a list of failures by public email at all. I hope it's not my
>> memory failing me here, because then I would have to remember why I
>> didn't speak up against that idea at the time.
>
> You didn't decide anything. As the CFM, I did. My job for this month
> is to make sure that 100% of patches in that queue get reviewed and
> either committed or bounced by July 15th. I'm doing my job.
>
> I will be more than happy to resign as CFM and turn it over to someone
> else if people have a problem with it.

Let's not be hasty :)

I think JoshB is spot on in this. He sent previous private emails, and a
week later opened up the transparency so that everyone understood what
was going on.

What I find unfortunate is people are spending a bunch of time on this
argument which has been clearl thought out by Josh instead of reviewing
patches.

I repeat:

Leave your ego at the door. Josh is doing what could be considered one
of the most thankless (public) jobs in this project. How about we
support him in getting these patches taken care of instead of whining
about the fact that he called us out for not doing our jobs (reviewing
patches) in the first place.

Sincerely,

Joshua D. Drkae

--
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

>> I will be more than happy to resign as CFM and turn it over to someone
>> else if people have a problem with it.
>
> Heck, Josh. People have to be allowed to critize *a small part* of your
> work without you understanding it as a fundamental request to step back
> from being CFM.

Criticize, yes. Which I'm responding to.

But if this group votes that I am not to publish a public list of
submitters who aren't reviewing again, or otherwise votes to restrict my
ability to enforce the rules which the Developer Meetings have decided
on for CFs, I will resign as CFM. The job is too hard to do with one
hand tied behind my back. That's what I'm saying.

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

On Mon, Jun 24, 2013 at 10:54 PM, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
>
>>> I will be more than happy to resign as CFM and turn it over to someone
>>> else if people have a problem with it.
>>
>> Heck, Josh. People have to be allowed to critize *a small part* of your
>> work without you understanding it as a fundamental request to step back
>> from being CFM.
>
> Criticize, yes. Which I'm responding to.
>
> But if this group votes that I am not to publish a public list of
> submitters who aren't reviewing again, or otherwise votes to restrict my
> ability to enforce the rules which the Developer Meetings have decided
> on for CFs, I will resign as CFM. The job is too hard to do with one
> hand tied behind my back. That's what I'm saying.

Hi Josh,

Not sure if this is out of line, but I would be glad to help you out
in any way possible for this CF. Please let me know if I can lighten
your burden in any way.

Regards,

Atri

--
Regards,

Atri
l'apprenant

On Mon, Jun 24, 2013 at 2:22 PM, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
> I have previously proposed that all of the reviewers of a given
> PostgreSQL release be honored in the release notes as a positive
> incentive, and was denied on this from doing so. Not coincidentally, we
> don't seem to have any reviewers-at-large anymore.

You know... that's a very good idea.

On 06/24/2013 10:22 AM, Josh Berkus wrote:

> Mind you, we wouldn't be able to reward a few reviewers, because they
> live in countries to which it's impossible to ship from abroad.
>
> I have previously proposed that all of the reviewers of a given
> PostgreSQL release be honored in the release notes as a positive
> incentive, and was denied on this from doing so. Not coincidentally, we
> don't seem to have any reviewers-at-large anymore.

I don't like idea of sending gifts. I do like the idea of public thanks.
We should put full recognition in the release notes for someone who
reviews a patch. If they didn't review the patch, the person that wrote
the patch would not have gotten the patch committed anyway. Writing the
patch is only have the battle.

Heck, think about the FKLocks patch, Alvaro wrote that patch but I know
that Noah (as well as others) put a herculean effort into helping get it
committed.

Reviewer recognition should be on the same level as the submitter.

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

JD said:
> Leave your ego at the door. Josh is doing what could be considered one
> of the most thankless (public) jobs in this project. How about we
> support him in getting these patches taken care of instead of whining
> about the fact that he called us out for not doing our jobs (reviewing
> patches) in the first place.

Actually, I think this is a very important thing for us to discuss, in
fact more important than reviewing any individual patch. 9.3 CF4 took
almost **4 months** so it's clear that the system isn't working as
designed. So let's hash this out during CF1 rather than during CF4.

We've had the policy of "submit one, review one" since the 9.2 dev
cycle. However, to my knowledge nobody has actually tried to enforce
this before, or even published stats on it. Once I did that for this
CF, it became readily apparent to me that the failure of this policy is
at least 50% of the problem with finishing commitfests -- and releases
-- on time.

The vast majority of submitters aren't reviewing other people's patches,
even ones who have the time and resources to do so. You'll notice that
most of the people on the List aren't new contributors to PostgreSQL; if
anything, the new contributors have been exemplary in responding to
private email that they need to do some review.

More, on the slacker list are 6-8 people who I happen to know are paid
by their employers to work on PostgreSQL. Those are the folks I'm
particularly targeting with the Slacker list; I want to make it
transparently clear to those folks' bosses that they have to give their
staff time for patch review if they expect to get the features *they*
want into PostgreSQL.

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

On 2013-06-24 10:37:02 -0700, Joshua D. Drake wrote:
>
> On 06/24/2013 10:22 AM, Josh Berkus wrote:
>
> >Mind you, we wouldn't be able to reward a few reviewers, because they
> >live in countries to which it's impossible to ship from abroad.
> >
> >I have previously proposed that all of the reviewers of a given
> >PostgreSQL release be honored in the release notes as a positive
> >incentive, and was denied on this from doing so. Not coincidentally, we
> >don't seem to have any reviewers-at-large anymore.
>
> I don't like idea of sending gifts. I do like the idea of public thanks. We
> should put full recognition in the release notes for someone who reviews a
> patch. If they didn't review the patch, the person that wrote the patch
> would not have gotten the patch committed anyway. Writing the patch is only
> have the battle.
>
> Heck, think about the FKLocks patch, Alvaro wrote that patch but I know that
> Noah (as well as others) put a herculean effort into helping get it
> committed.
>
> Reviewer recognition should be on the same level as the submitter.

The problem with that is that that HUGELY depends on the patch and the
review. There are patches where reviewers do a good percentage of the
work and others where they mostly tell that "compiles & runs".

Greetings,

Andres Freund

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

On Mon, Jun 24, 2013 at 12:57 PM, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
> Actually, every submitter on that list -- including Maciej -- was sent a
> personal, private email a week ago. A few (3) chose to take the
> opportunity to review things, or promised to do so, including a brand
> new Chinese contributor who needed help with English to review his
> patch. The vast majority chose not to respond to my email to them at
> all. When private email fails, the next step is public email.

Hrm, I'm on the slackers list, and I didn't see an email directed to
me from JB in the last week about the CF.

Anyway, I am hoping to take at least one patch this CF, though the
recent "review it within 5 days or else" policy combined with a ton of
my own work has kept me back so far.

Josh

On 24 June 2013 18:10, Josh Berkus <josh(at)agliodbs(dot)com> wrote:

> I will be more than happy to resign as CFM and turn it over to someone
> else if people have a problem with it.

Please don't do that (until at least the end of the CF ;-) )

It's a difficult job and I'm happy you're doing it, though I suggest
an optimal setting of 2-3 is likely to work best:
http://en.wikipedia.org/wiki/Mean_Machine_Angel

I would guess that many people probably don't actually understand the
problems or the policies that have emerged as a result.

Anyway, I won't say anymore because I'll probably be on some list or
other myself sometime.

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

> Hrm, I'm on the slackers list, and I didn't see an email directed to
> me from JB in the last week about the CF.

Really? Hmmm. I'm going to send you a test email privately, please
verify whether or not you get it.

> Anyway, I am hoping to take at least one patch this CF, though the
> recent "review it within 5 days or else" policy combined with a ton of
> my own work has kept me back so far.

Thanks! Your help is much appreciated, on whatever schedule you can do it!

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

On Mon, Jun 24, 2013 at 2:41 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
>> I don't like idea of sending gifts. I do like the idea of public thanks. We
>> should put full recognition in the release notes for someone who reviews a
>> patch. If they didn't review the patch, the person that wrote the patch
>> would not have gotten the patch committed anyway. Writing the patch is only
>> have the battle.
>>
>> Heck, think about the FKLocks patch, Alvaro wrote that patch but I know that
>> Noah (as well as others) put a herculean effort into helping get it
>> committed.
>>
>> Reviewer recognition should be on the same level as the submitter.
>
> The problem with that is that that HUGELY depends on the patch and the
> review. There are patches where reviewers do a good percentage of the
> work and others where they mostly tell that "compiles & runs".

Well, you can't so arbitrarily pick who you're recognizing as
contributor and who you aren't. So why not mention them all? They did
work for it, some more than others, but they all worked. And since
whoever submitted a patch (and got it committed) must have reviewed
something as well, they'd be recognized for both reviewing and
submitting.

> The problem with that is that that HUGELY depends on the patch and the
> review. There are patches where reviewers do a good percentage of the
> work and others where they mostly tell that "compiles & runs".

This project is enormously stingy with giving credit to people. It's
not like it costs us money, you know.

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

On 06/24/2013 10:48 AM, Claudio Freire wrote:

>>> Reviewer recognition should be on the same level as the submitter.
>>
>> The problem with that is that that HUGELY depends on the patch and the
>> review. There are patches where reviewers do a good percentage of the
>> work and others where they mostly tell that "compiles & runs".
>
>
> Well, you can't so arbitrarily pick who you're recognizing as
> contributor and who you aren't. So why not mention them all? They did
> work for it, some more than others, but they all worked. And since
> whoever submitted a patch (and got it committed) must have reviewed
> something as well, they'd be recognized for both reviewing and
> submitting.
>

Exactly. Just make it a simple policy:

Submitters and Reviewers are listed in that order:

Submitter, reviewer, reviewer

That way submitter gets first bill, satisfying the ego (as well as
professional consideration) but reviewers are also fully recognized.

Sincerely,

Joshua D. Drkae

--
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-24 14:48:32 -0300, Claudio Freire wrote:
> On Mon, Jun 24, 2013 at 2:41 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> >> I don't like idea of sending gifts. I do like the idea of public thanks. We
> >> should put full recognition in the release notes for someone who reviews a
> >> patch. If they didn't review the patch, the person that wrote the patch
> >> would not have gotten the patch committed anyway. Writing the patch is only
> >> have the battle.
> >>
> >> Heck, think about the FKLocks patch, Alvaro wrote that patch but I know that
> >> Noah (as well as others) put a herculean effort into helping get it
> >> committed.
> >>
> >> Reviewer recognition should be on the same level as the submitter.
> >
> > The problem with that is that that HUGELY depends on the patch and the
> > review. There are patches where reviewers do a good percentage of the
> > work and others where they mostly tell that "compiles & runs".
>
>
> Well, you can't so arbitrarily pick who you're recognizing as
> contributor and who you aren't. So why not mention them all? They did
> work for it, some more than others, but they all worked. And since
> whoever submitted a patch (and got it committed) must have reviewed
> something as well, they'd be recognized for both reviewing and
> submitting.

Because spending a year working on a feature isn't the same as spending
an hour or day on it. And the proposal was to generally list them at the
same level.
At least the 9.3 release notes seem to list people that reviewed
extensively prominently on the patches...

Greetings,

Andres Freund

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

> Because spending a year working on a feature isn't the same as spending
> an hour or day on it. And the proposal was to generally list them at the
> same level.
> At least the 9.3 release notes seem to list people that reviewed
> extensively prominently on the patches...

My proposal was to have a compiled list of reviewers at the end of the
release notes, in the form of:

"Reviewers and Testers for #.# Included: name, name, name, name"

That way we can pick up the trivial reviewers as well, and even testers
who are not otherwise contributors.

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

On 2013-06-24 10:50:42 -0700, Josh Berkus wrote:
>
> > The problem with that is that that HUGELY depends on the patch and the
> > review. There are patches where reviewers do a good percentage of the
> > work and others where they mostly tell that "compiles & runs".
>
> This project is enormously stingy with giving credit to people. It's
> not like it costs us money, you know.

Listing a reviewer that didn't do all that much at the same level as the
author or an somebody having done an extensive review will cost you
contributors in the long run.

I am all for introducing a "Contributed by reviewing: ..." section in
the release notes.

Greetings,

Andres Freund

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

On 06/24/2013 10:59 AM, Andres Freund wrote:
>
> On 2013-06-24 10:50:42 -0700, Josh Berkus wrote:
>>
>>> The problem with that is that that HUGELY depends on the patch and the
>>> review. There are patches where reviewers do a good percentage of the
>>> work and others where they mostly tell that "compiles & runs".
>>
>> This project is enormously stingy with giving credit to people. It's
>> not like it costs us money, you know.
>
> Listing a reviewer that didn't do all that much at the same level as the
> author or an somebody having done an extensive review will cost you
> contributors in the long run.
>
> I am all for introducing a "Contributed by reviewing: ..." section in
> the release notes.

It should be listed with the specific feature.

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 Mon, Jun 24, 2013 at 1:24 PM, Joshua D. Drake <jd(at)commandprompt(dot)com> wrote:
> Leave your ego at the door. Josh is doing what could be considered one of
> the most thankless (public) jobs in this project. How about we support him
> in getting these patches taken care of instead of whining about the fact
> that he called us out for not doing our jobs (reviewing patches) in the
> first place.

Quite.

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

I'm just wondering about newbies...

I've created my first patch, so I'm one of them, I think.

I've reviewed some patches, but only some easier ones, like pure regression
tests. Unfortunately my knowledge is not enough to review patches making
very deep internal changes, or some efficiency tweaks. I'm not sure if
those patches should be reviewed by newbies like me, as I just don't know
what is good and what is bad, even if a patch looks OK for me. What's the
use of my review, if I don't understand the internals enough, I can apply
the patch, run tests, look inside and I'm sure I won't find any problems?

Maybe this is the reason why there are not so many reviewers?

I'm not sure if such a strict policy will bring anything good. If newbies
won't be able to review patches, they won't be committing simple patches,
as they won't be able to review others.

If this policy will be so strict, I will spend huge amount of time to
understand the whole Postgres code before sending my next patch, as most
probably I will have problems with making the reviews.

Szymon

Szymon,

> I've reviewed some patches, but only some easier ones, like pure regression
> tests.

Actually, you were one of the people I was thinking of when I said
"mostly the new submitters have been exemplary in claiming some review
work". You're helping a lot.

> Unfortunately my knowledge is not enough to review patches making
> very deep internal changes, or some efficiency tweaks. I'm not sure if
> those patches should be reviewed by newbies like me, as I just don't know
> what is good and what is bad, even if a patch looks OK for me. What's the
> use of my review, if I don't understand the internals enough, I can apply
> the patch, run tests, look inside and I'm sure I won't find any problems?

Actually, something like 50% of all patches get sent back to the
submitter on the basis of a build/test/functionality check/completeness
check/standards compliance/do we really want this?. The fact that you
are doing those steps instead of a committer, and thus letting the
committer look at 50% fewer patches, *does* help.

In fact, the single most important part of the regression test reviews
is "does this new regression test test anything worthwhile?" and "does
this regression test return the same results on different machines?".
You already know enough to address both of those questions, at least
enough to bring up any potential problems on this list.

In the future, I would like to have automated systems do the
apply/build/regression test check, leaving new reviewers to check only
functionality and completeness and other things which require a human.
But we don't have the technology for that yet.

> Maybe this is the reason why there are not so many reviewers?
>
> I'm not sure if such a strict policy will bring anything good. If newbies
> won't be able to review patches, they won't be committing simple patches,
> as they won't be able to review others.

Commit a simple patch, review a simple patch. If we have 20 submitters
of simple patches, then we have 20 simple patches to review, no?

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

On Mon, Jun 24, 2013 at 10:40:48AM -0700, Josh Berkus wrote:
> More, on the slacker list are 6-8 people who I happen to know are paid
> by their employers to work on PostgreSQL. Those are the folks I'm
> particularly targeting with the Slacker list; I want to make it
> transparently clear to those folks' bosses that they have to give their
> staff time for patch review if they expect to get the features *they*
> want into PostgreSQL.

I am confused. Why is everyone interpreting "slacker" negatively. ;-) LOL

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

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

On 25/06/13 03:54, Joshua D. Drake wrote:
>
>
> It is mentioned. Of course now I can't find it but it is there.
>
> However, I believe you are taking the wrong perspective on this. This is
> not a shame wall. It is a transparent reminder of the policy and those
> who have not assisted in reviewing a patch but have submitted a patch
> themselves.
>
> In short, leave the ego at the door.
>

Lol - Josh's choice of title has made it a small shame wall (maybe only
knee high).

However as your last line says - no *actual* harm has been done (no
kittens killed etc).

One of the reasons for fewer reviewers than submitters, is that it is a
fundamentally more difficult job. I've submitted a few patches in a few
different areas over the years - however if I grab a patch on the queue
that is not in exactly one of the areas I know about, I'll struggle to
do a good quality review.

Now some might say "any review is better than no review"... I don't
think so - one of my patches a while was reviewed by someone who didn't
really know the context that well and made the whole process grind to a
standstill until a more experienced reviewer took over. I'm quite wary
of doing the same myself - anti-help is not the answer!

Regards

Mark

On Mon, Jun 24, 2013 at 10:10:11AM -0700, Josh Berkus wrote:
> On 06/24/2013 10:02 AM, Dimitri Fontaine wrote:
> > Josh Berkus <josh(at)agliodbs(dot)com> writes:
> >> patch. The vast majority chose not to respond to my email to them at
> >> all. When private email fails, the next step is public email.
> >
> > The only problem I have here is that I don't remember about deciding to
> > publish a list of failures by public email at all. I hope it's not my
> > memory failing me here, because then I would have to remember why I
> > didn't speak up against that idea at the time.
>
> You didn't decide anything. As the CFM, I did. My job for this month
> is to make sure that 100% of patches in that queue get reviewed and
> either committed or bounced by July 15th. I'm doing my job.

+1 for trying the management practices you're trying. After the CF is closed,
we can step back and treat ourselves to a nice debate about them.

Thanks,
nm

--
Noah Misch
EnterpriseDB http://www.enterprisedb.com

On Tue, Jun 25, 2013 at 11:06 AM, Noah Misch <noah(at)leadboat(dot)com> wrote:
> On Mon, Jun 24, 2013 at 10:10:11AM -0700, Josh Berkus wrote:
>> On 06/24/2013 10:02 AM, Dimitri Fontaine wrote:
>> > Josh Berkus <josh(at)agliodbs(dot)com> writes:
>> >> patch. The vast majority chose not to respond to my email to them at
>> >> all. When private email fails, the next step is public email.
>> >
>> > The only problem I have here is that I don't remember about deciding to
>> > publish a list of failures by public email at all. I hope it's not my
>> > memory failing me here, because then I would have to remember why I
>> > didn't speak up against that idea at the time.
>>
>> You didn't decide anything. As the CFM, I did. My job for this month
>> is to make sure that 100% of patches in that queue get reviewed and
>> either committed or bounced by July 15th. I'm doing my job.
>
> +1 for trying the management practices you're trying. After the CF is closed,
> we can step back and treat ourselves to a nice debate about them.
Same here. +1. This method is good to gather in a single, short email
all the information a common user cannot see by watching the
commitfest page, becoming hard to understand globally with a growing
number of pending patches.
--
Michael

Mark Kirkwood <mark(dot)kirkwood(at)catalyst(dot)net(dot)nz> writes:
> One of the reasons for fewer reviewers than submitters, is that it is a
> fundamentally more difficult job. I've submitted a few patches in a few
> different areas over the years - however if I grab a patch on the queue
> that is not in exactly one of the areas I know about, I'll struggle to
> do a good quality review.

> Now some might say "any review is better than no review"... I don't
> think so - one of my patches a while was reviewed by someone who didn't
> really know the context that well and made the whole process grind to a
> standstill until a more experienced reviewer took over. I'm quite wary
> of doing the same myself - anti-help is not the answer!

FWIW, a large part of the reason for the commitfest structure is that
by reviewing patches, people can educate themselves about parts of the
PG code that they don't know already, and thus become better qualified
to do more stuff later. So I've got no problem with less-experienced
people doing reviews.

At the same time, it *is* fair to expect someone to phrase their review
as "I don't understand this, could you explain and/or improve the
comments" rather than saying something more negative, if they aren't
clear about what's going on. Without some specific references it's hard
to be sure if the reviewer you mention was being unreasonable.

Anyway, the point I'm trying to make is that this is a community effort,
and each of us can stand to improve our knowledge of what is fundamentally
a complex system. Learn something, teach something, it's all good.

regards, tom lane

On 25/06/13 15:56, Tom Lane wrote:
> Mark Kirkwood <mark(dot)kirkwood(at)catalyst(dot)net(dot)nz> writes:
>> One of the reasons for fewer reviewers than submitters, is that it is a
>> fundamentally more difficult job. I've submitted a few patches in a few
>> different areas over the years - however if I grab a patch on the queue
>> that is not in exactly one of the areas I know about, I'll struggle to
>> do a good quality review.
>
>> Now some might say "any review is better than no review"... I don't
>> think so - one of my patches a while was reviewed by someone who didn't
>> really know the context that well and made the whole process grind to a
>> standstill until a more experienced reviewer took over. I'm quite wary
>> of doing the same myself - anti-help is not the answer!
>
> FWIW, a large part of the reason for the commitfest structure is that
> by reviewing patches, people can educate themselves about parts of the
> PG code that they don't know already, and thus become better qualified
> to do more stuff later. So I've got no problem with less-experienced
> people doing reviews.
>
> At the same time, it *is* fair to expect someone to phrase their review
> as "I don't understand this, could you explain and/or improve the
> comments" rather than saying something more negative, if they aren't
> clear about what's going on. Without some specific references it's hard
> to be sure if the reviewer you mention was being unreasonable.
>
> Anyway, the point I'm trying to make is that this is a community effort,
> and each of us can stand to improve our knowledge of what is fundamentally
> a complex system. Learn something, teach something, it's all good.
>

Yes - the reason I mentioned this was not to dig into history and bash a
reviewer (who was not at all unreasonable in my recollection)... but to
highlight that approaching a review is perhaps a little more complex and
demanding that was being made out, hence the shortage of volunteers.

However I do completely agree, that encouraging reviewers to proceed
with the approach you've outlined above seems like "the way". And yes -
it is going to be a good way to get to know the code better.

Regards

Mark

On Tue, Jun 25, 2013 at 3:56 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

>
> FWIW, a large part of the reason for the commitfest structure is that
> by reviewing patches, people can educate themselves about parts of the
> PG code that they don't know already, and thus become better qualified
> to do more stuff later. So I've got no problem with less-experienced
> people doing reviews.
>
> At the same time, it *is* fair to expect someone to phrase their review
> as "I don't understand this, could you explain and/or improve the
> comments" rather than saying something more negative, if they aren't
> clear about what's going on. Without some specific references it's hard
> to be sure if the reviewer you mention was being unreasonable.
>
> Anyway, the point I'm trying to make is that this is a community effort,
> and each of us can stand to improve our knowledge of what is fundamentally
> a complex system. Learn something, teach something, it's all good.
>
> regards, tom lane
>
>
>
I just wanted to give this the +1 but also want to add. As a novice back in
the 8.4 cycle I wrote a small patch implement boyer-moore-horspool text
searches. I didn't have too much experience around the PostgreSQL source
code, so when it came to my review of another patch (which I think was even
the rule back in 8.4 IIRC) I was quite clear on what I could and could not
review. The initial windowing function patch was in the queue at the time,
so I picked that one and reviewed it along with Heikki. As a novice I did
manage to help maintain a bit of concurrency with the progress of the patch
and the patch went through quite a few revisions from my review even before
Heikki got a good look at it. I think the most important thing is
maintaining that concurrency during the commitfest, if the author of the
patch is sitting idle waiting for a review the whole time then that leaves
so much less time to get the patch into shape before the commiter comes
along. Even if a novice reviewer can only help a tiny bit, it might still
make the difference between that patch making it to a suitable state in
time or it getting bounced to the next commitfest or even the next release.

So for a person who is a little scared to put their name in the reviewer
section of a patch, I'd recommend being quite open and honest with what you
can and can't review. For me back in 8.4 with the windowing function patch,
I managed point out a few places were the plans being created where not
quite optimal and the author of the patch quickly put fixes in and sent
updated patches, there was also a few things around the SQL spec that I
found after grabbing a copy of the spec and reading part of it. It may have
been a small part of the overall review and work to get the patch commited
but as Tom stated, I did learn quite a bit from that and I even managed to
sent back a delta patch which helped to get the patch more SQL spec
compliant.

I'm not sure if adding any a review breakdown list to the commitfest
application would be of any use to allow breaking down of what the reviewer
is actually reviewing. Perhaps people would be quicker to sign up to review
the sections of patches around their area of expertise rather than putting
their name against the whole thing, likely a commiter would have a better
idea if such a thing was worth the extra overhead.

Regards

David Rowley

On 25 June 2013 04:13, Joshua D. Drake <jd(at)commandprompt(dot)com> wrote:
> On 06/24/2013 10:59 AM, Andres Freund wrote:
>> On 2013-06-24 10:50:42 -0700, Josh Berkus wrote:
>>> This project is enormously stingy with giving credit to people. It's
>>> not like it costs us money, you know.
>> I am all for introducing a "Contributed by reviewing: ..." section in
>> the release notes.
>
> It should be listed with the specific feature.

I don't have a strong opinion about whether the reviewers ought to be
listed all together or with each feature, but I do feel very strongly
that they should be given some kind of credit.

Reviewing is often not all that much fun (compared with authoring) and
as Josh points out, giving proper credit has a "bang for buck"
incentive value that is hard to argue with.

Cheers,
BJ

Hi,

Apologies for being unable to respond promptly. I've been traveling
(without much access) and this was the fastest I could settle down. I was
free for months and had to travel smack in the middle of the commitfest.

Incidentally I had reviewed one patch after your direct email, but as
someone earlier mentioned, actually pasting my name as 'reviewer' seemed
awkward and so didn't
then
(but currently its set to David Fetter for some reason).

I guess the point Tom mentioned earlier makes good sense and I agree with
the
spirit of the
process
.
In my part would try to review more patches and mark them so on the
commitfest page
soon
.
--
Robins Tharakan

On 23 June 2013 23:41, Josh Berkus <josh(at)agliodbs(dot)com> wrote:

> Folks,
>
> For 9.2, we adopted it as policy that anyone submitting a patch to a
> commitfest is expected to review at least one patch submitted by someone
> else. And that failure to do so would affect the attention your patches
> received in the future. For that reason, I'm publishing the list below
> of people who submitted patches and have not yet claimed any patch in
> the commitfest to review.
>
> For those of you who are contributing patches for your company, please
> let your boss know that reviewing is part of contributing, and that if
> you don't do the one you may not be able to do the other.
>
> The two lists below, idle submitters and committers, constitutes 26
> people. This *outnumbers* the list of contributors who are busy
> reviewing patches -- some of them four or more patches. If each of
> these people took just *one* patch to review, it would almost entirely
> clear the list of patches which do not have a reviewer. If these folks
> continue not to do reviews, this commitfest will drag on for at least 2
> weeks past its closure date.
>
> Andrew Geirth
> Nick White
> Peter Eisentrout
> Alexander Korotkov
> Etsuro Fujita
> Hari Babu
> Jameison Martin
> Jon Nelson
> Oleg Bartunov
> Chris Farmiloe
> Samrat Revagade
> Alexander Lakhin
> Mark Kirkwood
> Liming Hu
> Maciej Gajewski
> Josh Kuperschmidt
> Mark Wong
> Gurjeet Singh
> Robins Tharakan
> Tatsuo Ishii
> Karl O Pinc
>
> Additionally, the following committers are not listed as reviewers on
> any patch. Note that I have no way to search which ones might be
> *committers* on a patch, so these folks are not necessarily slackers
> (although with Bruce, we know for sure):
>
> Bruce Momjian (momjian)
> Michael Meskes (meskes)
> Teodor Sigaev (teodor)
> Andrew Dunstan (adunstan)
> Magnus Hagander (mha)
> Itagaki Takahiro (itagaki)
>
> --
> Josh Berkus
> PostgreSQL Experts Inc.
> http://pgexperts.com
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>

Sorry for joining the thread this late, but I didn't really expect to see
myself listed as a slacker on a public list.

> Additionally, the following committers are not listed as reviewers on
> any patch. Note that I have no way to search which ones might be
> *committers* on a patch, so these folks are not necessarily slackers

This means I'm a slacker because I'm not reviewing or committing a patch,
right? Do we have a written rule somewhere? Or some other communication about
this? I would have liked to know this requirement before getting singled out in
public.

Also I'd like to know who made the decision to require a patch review from each
committer as I certainly missed it. Was the process public? Where can I find
more about it? In general I find it difficult to digest that other people make
decisions about my spare time without me having a word in the discussion.

Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
Jabber: michael.meskes at gmail dot com
VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL

> Folks,
>
> For 9.2, we adopted it as policy that anyone submitting a patch to a
> commitfest is expected to review at least one patch submitted by someone
> else. And that failure to do so would affect the attention your patches
> received in the future. For that reason, I'm publishing the list below
> of people who submitted patches and have not yet claimed any patch in
> the commitfest to review.
>
> For those of you who are contributing patches for your company, please
> let your boss know that reviewing is part of contributing, and that if
> you don't do the one you may not be able to do the other.
>
> The two lists below, idle submitters and committers, constitutes 26
> people. This *outnumbers* the list of contributors who are busy
> reviewing patches -- some of them four or more patches. If each of
> these people took just *one* patch to review, it would almost entirely
> clear the list of patches which do not have a reviewer. If these folks
> continue not to do reviews, this commitfest will drag on for at least 2
> weeks past its closure date.
>
> Andrew Geirth
> Nick White
> Peter Eisentrout
> Alexander Korotkov
> Etsuro Fujita
> Hari Babu
> Jameison Martin
> Jon Nelson
> Oleg Bartunov
> Chris Farmiloe
> Samrat Revagade
> Alexander Lakhin
> Mark Kirkwood
> Liming Hu
> Maciej Gajewski
> Josh Kuperschmidt
> Mark Wong
> Gurjeet Singh
> Robins Tharakan
> Tatsuo Ishii
> Karl O Pinc

It took me for while before realizing that I am on the list because I
posted this:

http://www.postgresql.org/message-id/20130610.091605.250603479334631505.t-ishii@sraoss.co.jp

Because I did not register the patch into CF page myself. I should
have not posted it until I find any patch which I can take care
of. Sorry for this.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp

On Tue, Jul 2, 2013 at 10:52:26AM +0200, Michael Meskes wrote:
> Sorry for joining the thread this late, but I didn't really expect to see
> myself listed as a slacker on a public list.
>
> > Additionally, the following committers are not listed as reviewers on
> > any patch. Note that I have no way to search which ones might be
> > *committers* on a patch, so these folks are not necessarily slackers
>
> This means I'm a slacker because I'm not reviewing or committing a patch,
> right? Do we have a written rule somewhere? Or some other communication about
> this? I would have liked to know this requirement before getting singled out in
> public.
>
> Also I'd like to know who made the decision to require a patch review from each
> committer as I certainly missed it. Was the process public? Where can I find
> more about it? In general I find it difficult to digest that other people make
> decisions about my spare time without me having a word in the discussion.

I understand. You could wear "slacker" as a badge of honor: ;-)

http://momjian.us/main/img/main/slacker.jpg

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

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

On 07/02/2013 11:30 AM, Tatsuo Ishii wrote:
>> Folks,
>>
>> For 9.2, we adopted it as policy that anyone submitting a patch to a
>> commitfest is expected to review at least one patch submitted by someone
>> else. And that failure to do so would affect the attention your patches
>> received in the future. For that reason, I'm publishing the list below
>> of people who submitted patches and have not yet claimed any patch in
>> the commitfest to review.
>>
>> For those of you who are contributing patches for your company, please
>> let your boss know that reviewing is part of contributing, and that if
>> you don't do the one you may not be able to do the other.
>>
>> The two lists below, idle submitters and committers, constitutes 26
>> people. This *outnumbers* the list of contributors who are busy
>> reviewing patches -- some of them four or more patches. If each of
>> these people took just *one* patch to review, it would almost entirely
>> clear the list of patches which do not have a reviewer. If these folks
>> continue not to do reviews, this commitfest will drag on for at least 2
>> weeks past its closure date.
>>
>> Andrew Geirth
>> Nick White
>> Peter Eisentrout
>> Alexander Korotkov
>> Etsuro Fujita
>> Hari Babu
>> Jameison Martin
>> Jon Nelson
>> Oleg Bartunov
>> Chris Farmiloe
>> Samrat Revagade
>> Alexander Lakhin
>> Mark Kirkwood
>> Liming Hu
>> Maciej Gajewski
>> Josh Kuperschmidt
>> Mark Wong
>> Gurjeet Singh
>> Robins Tharakan
>> Tatsuo Ishii
>> Karl O Pinc
> It took me for while before realizing that I am on the list because I
> posted this:
>
> http://www.postgresql.org/message-id/20130610.091605.250603479334631505.t-ishii@sraoss.co.jp
>
> Because I did not register the patch into CF page myself. I should
> have not posted it until I find any patch which I can take care
> of.
Hi Tatsuo-san

I guess whoever registered it with CF should also take your place on the
slackers list ;)

Regards
Hannu Krosing

PS. I am also currently witholding a patch from CF for the same reason

> Sorry for this.
> --
> Tatsuo Ishii
> SRA OSS, Inc. Japan
> English: http://www.sraoss.co.jp/index_en.php
> Japanese: http://www.sraoss.co.jp
>
>

On Tue, Jul 2, 2013 at 3:00 PM, Hannu Krosing <hannu(at)krosing(dot)net> wrote:
> I guess whoever registered it with CF should also take your place on the
> slackers list ;)

Yeah, I recommend that, in the future, CF managers do NOT go and add
patches to the CF. Pinging newbies to see if they just forgot is
sensible, but if an experienced hacker hasn't put something in the CF,
there's probably a reason.

Also, I recommend that nobody get too worked up about being on the
slacker list. Life is short, PostgreSQL is awesome, and nobody can
make you review patches against your will. Don't take it for more
than what Josh meant it as.

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

On 2013/07/02, at 23:44, Bruce Momjian <bruce(at)momjian(dot)us> wrote:

> I understand. You could wear "slacker" as a badge of honor: ;-)
> http://momjian.us/main/img/main/slacker.jpg
This picture could make a nice T-shirt btw.
>
--
Michael

Hackers,

Clearly I ticked off a bunch of people by publishing "the list". On the
other hand, in the 5 days succeeding the post, more than a dozen
additional people signed up to review patches, and we got some of the
"ready for committer" patches cleared out -- something which nothing
else I did, including dozens of private emails, general pleas to this
mailing list, mails to the RRReviewers list, served to accomplish, in
this or previous CFs.

So, as an experiment, call it a mixed result. I would like to have some
other way to motivate reviewers than public shame. I'd like to have
some positive motivations for reviewers, such as public recognition by
our project and respect from hackers, but I'm doubting that those are
actually going to happen, given the feedback I've gotten on this list to
the idea.

I do think I succeeded in calling attention to the fact that this
project has slid into a rut of letting a handful of people do 90% of the
reviewing, resulting in CFs which last forever and some very frustrated
major contributors. That part shouldn't be necessary again for some
time, hopefully.

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

On Tue, Jul 02, 2013 at 04:00:22PM -0400, Robert Haas wrote:
> make you review patches against your will. Don't take it for more
> than what Josh meant it as.

And that was what?

Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
Jabber: michael.meskes at gmail dot com
VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL

On Tue, Jul 02, 2013 at 09:42:43PM -0700, Josh Berkus wrote:
> Clearly I ticked off a bunch of people by publishing "the list". On the
> other hand, in the 5 days succeeding the post, more than a dozen
> additional people signed up to review patches, and we got some of the
> "ready for committer" patches cleared out -- something which nothing
> else I did, including dozens of private emails, general pleas to this
> mailing list, mails to the RRReviewers list, served to accomplish, in
> this or previous CFs.

So your saying the end justifies the means? I beg to disagree.

> So, as an experiment, call it a mixed result. I would like to have some
> other way to motivate reviewers than public shame. I'd like to have

Doesn't shame imply that people knew that were supposed to review patches in
the first place? An implication that is not true, at least for some on your
list. I think I better not bring up the word I would describe your email with,
just for the fear of mistranslating it.

Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
Jabber: michael.meskes at gmail dot com
VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL

On Wed, Jul 3, 2013 at 9:21 AM, Michael Meskes <meskes(at)postgresql(dot)org> wrote:
> On Tue, Jul 02, 2013 at 04:00:22PM -0400, Robert Haas wrote:
>> make you review patches against your will. Don't take it for more
>> than what Josh meant it as.
>
> And that was what?

An attempt to prod a few more people into helping review.

I can see that this pissed you off, and I'm sorry about that. But I
don't think that was his intent.

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

> Clearly I ticked off a bunch of people by publishing "the list". On the
> other hand, in the 5 days succeeding the post, more than a dozen
> additional people signed up to review patches, and we got some of the
> "ready for committer" patches cleared out -- something which nothing
> else I did, including dozens of private emails, general pleas to this
> mailing list, mails to the RRReviewers list, served to accomplish, in
> this or previous CFs.

Others rules appeared, like the 5 days limit.
To me it outlines that some are abusing the CF app and pushing there useless
patches (not still ready or complete, WIP, ...)

> So, as an experiment, call it a mixed result. I would like to have some
> other way to motivate reviewers than public shame. I'd like to have
> some positive motivations for reviewers, such as public recognition by
> our project and respect from hackers, but I'm doubting that those are
> actually going to happen, given the feedback I've gotten on this list to
> the idea.

You're looking at a short term, big effect.
And long term ? Will people listed still be interested to participate in a
project which stamps people ?

With or without review, it's a shame if people stop proposing patches because
they are not sure to get time to review other things *in time*.
--
Cédric Villemain +33 (0)6 20 30 22 52
http://2ndQuadrant.fr/
PostgreSQL: Support 24x7 - Développement, Expertise et Formation

On Wed, Jul 3, 2013 at 2:24 PM, Cédric Villemain <cedric(at)2ndquadrant(dot)com>wrote:

> > Clearly I ticked off a bunch of people by publishing "the list". On the
> > other hand, in the 5 days succeeding the post, more than a dozen
> > additional people signed up to review patches, and we got some of the
> > "ready for committer" patches cleared out -- something which nothing
> > else I did, including dozens of private emails, general pleas to this
> > mailing list, mails to the RRReviewers list, served to accomplish, in
> > this or previous CFs.
>
> Others rules appeared, like the 5 days limit.
> To me it outlines that some are abusing the CF app and pushing there
> useless
> patches (not still ready or complete, WIP, ...

Seems to me that "useless" overstates things, but it does seem fair to
say that some patches are not sufficiently well prepared to be efficiently
added into Postgres.

> So, as an experiment, call it a mixed result. I would like to have some
> > other way to motivate reviewers than public shame. I'd like to have
> > some positive motivations for reviewers, such as public recognition by
> > our project and respect from hackers, but I'm doubting that those are
> > actually going to happen, given the feedback I've gotten on this list to
> > the idea.
>
> You're looking at a short term, big effect.
> And long term ? Will people listed still be interested to participate in a
> project which stamps people ?
>
> With or without review, it's a shame if people stop proposing patches
> because
> they are not sure to get time to review other things *in time*.

Well, if the project is hampered by not being able to get *all* the
changes that people imagine that they want to put in, then we have a
real problem of needing a sort of "triage" to determine which changes
will be accepted, and which will not.

Perhaps we need an extra status in the CommitFest application, namely
one that characterizes:
Insufficiently Important To Warrant Review

That's too long a term. Perhaps "Not Review-worthy" expresses it better?
--
When confronted by a difficult problem, solve it by reducing it to the
question, "How would the Lone Ranger handle this?"

Tatsuo,

> Because I did not register the patch into CF page myself. I should
> have not posted it until I find any patch which I can take care
> of. Sorry for this.

My apologies! I did post the list of patches I'd added to the CF in my
"patch sweep" to -hackers, but I forgot to match it against the list of
contributors who weren't reviewing. Sorry about that.

In general, I prefer to do the patch sweep 5 days out from the start of
the CF so that I have time to email people about whether or not their
patches should have been included. However, this time an emergency
cropped up just before the CF started and I found myself doing the patch
sweep the day before the CF, which didn't leave time for an email response.

This is one of those areas where better tooling could help a lot.

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

Michael Meskes wrote:
>> So, as an experiment, call it a mixed result. I would like to have some
>> other way to motivate reviewers than public shame. I'd like to have
>
> Doesn't shame imply that people knew that were supposed to review patches in
> the first place? An implication that is not true, at least for some on your
> list. I think I better not bring up the word I would describe your email with,
> just for the fear of mistranslating it.

If you didn't feel obligated, you wouldn't be pissed at me. You'd just
blow it off (like Bruce did). I think you're angry with me because you
feel guilty.

My *personal* viewpoint is that all committers should feel obligated to
review and commit patches from other contributors. That's why they're
committers in the first place. Certainly if a committer looks at the CF
application and notices that 80% of the reviewing and committing is
being done by three people, none of whom have any more "spare time" than
they do, they should feel obligated to help those people out.

We have a problem with patch reviewing and committing in this project;
it's not being done in a timely fashion in general (every CF last year
ended late), and the people who are doing most of the work feel
overworked and frustrated. This problem is getting worse every year,
and will kill the project if it continues on its current trajectory.

There are *only* three ways out of this hole, all three of which I'm
trying to address:

1) more automation and better tools in order to reduce the total time
required of each reviewer/committer;

2) a program of recruitment of new reviewers, including giving respect
and recognition to people for their reviewing efforts

3) getting most of our existing contributors to shoulder their fair
share of patch review.

(3) is what I'm addressing on this thread. The reason I volunteered to
be CFM this time was directly because of our discussion in Ottawa of how
the review process wasn't working. I decided to find out *why* it
wasn't working, and the first obvious thing I ran across was that most
of our current and our long-term contributors weren't doing any patch
review. For CF1, the number of people submitting patches outnumbered
those who had volunteered for review 2 to 1. That *is* the review
problem in a nutshell; everybody wants someone else to do the work.

I don't think it's too much to ask people who are listed on the project
developers page as major contributors to review one patch per CommitFest
most of the time. If they did just *one* it would substantially
decrease the workload on the people who are currently doing the vast
majority of review and commit.

On 07/03/2013 11:24 AM, Cédric Villemain wrote:
> You're looking at a short term, big effect.
> And long term ? Will people listed still be interested to participate
in a
> project which stamps people ?
>
> With or without review, it's a shame if people stop proposing patches
because
> they are not sure to get time to review other things *in time*.

Yes, I am, because the CF is only supposed to be 30 days long, and I
plan to finish it on time. That's my job as CFM.

Several people on this thread have raised the fear of discouraging patch
submitters, but the consistent evidence is that we have more submissions
than we can currently handle. I'd rather have half as many submissions,
but do a really good job of reviewing, improving, and integrating those
than the current mess.

Furthermore, there are quite a number of people who are submitting
patches on paid company time. For those people, "submit one, review
one" has to be an ironclad rule so that they can tell their bosses that
they *have* to spend time on patch review. Otherwise, the review
doesn't happen.

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

Le mercredi 3 juillet 2013 21:03:42, Christopher Browne a écrit :
> On Wed, Jul 3, 2013 at 2:24 PM, Cédric Villemain
<cedric(at)2ndquadrant(dot)com>wrote:
> > > Clearly I ticked off a bunch of people by publishing "the list". On
> > > the other hand, in the 5 days succeeding the post, more than a dozen
> > > additional people signed up to review patches, and we got some of the
> > > "ready for committer" patches cleared out -- something which nothing
> > > else I did, including dozens of private emails, general pleas to this
> > > mailing list, mails to the RRReviewers list, served to accomplish, in
> > > this or previous CFs.
> >
> > Others rules appeared, like the 5 days limit.
> > To me it outlines that some are abusing the CF app and pushing there
> > useless
> > patches (not still ready or complete, WIP, ...
>
> Seems to me that "useless" overstates things, but it does seem fair to
> say that some patches are not sufficiently well prepared to be efficiently
> added into Postgres.

Oops! You just made me realized my choice of words was not good at all!
I didn't considered under this angle, I just meant that some patches were
added to CF to help patches authors, it was a good idea, but this had some
unexpected corner case. Sorry for the confusion.
--
Cédric Villemain +33 (0)6 20 30 22 52
http://2ndQuadrant.fr/
PostgreSQL: Support 24x7 - Développement, Expertise et Formation

On Wed, Jul 3, 2013 at 12:34:50PM -0700, Josh Berkus wrote:
> Michael Meskes wrote:
> >> So, as an experiment, call it a mixed result. I would like to have some
> >> other way to motivate reviewers than public shame. I'd like to have
> >
> > Doesn't shame imply that people knew that were supposed to review patches in
> > the first place? An implication that is not true, at least for some on your
> > list. I think I better not bring up the word I would describe your email with,
> > just for the fear of mistranslating it.
>
> If you didn't feel obligated, you wouldn't be pissed at me. You'd just
> blow it off (like Bruce did). I think you're angry with me because you
> feel guilty.

People are supposed to feel guilty because they are not volunteering
enough time, or enough time in the places the community wants? How does
that make sense? Doesn't that contradict the term "volunteer"?

Also, don't assume everyone has the thick skin I do.

I do understand Josh's frustration that something different had to be
done.

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

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

On Wed, Jul 03, 2013 at 09:47:13AM -0400, Robert Haas wrote:
> An attempt to prod a few more people into helping review.
>
> I can see that this pissed you off, and I'm sorry about that. But I
> don't think that was his intent.

I hoped for this kind of answer from him but ...

Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
Jabber: michael.meskes at gmail dot com
VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL

On Wed, Jul 03, 2013 at 12:34:50PM -0700, Josh Berkus wrote:
> If you didn't feel obligated, you wouldn't be pissed at me. You'd just
> blow it off (like Bruce did). I think you're angry with me because you
> feel guilty.

That is outrageous bullshit!

> My *personal* viewpoint is that all committers should feel obligated to

And my *personal* viewpoint is that nobody should be offended like this. But
apparently I don't get my wish either.

> review and commit patches from other contributors. That's why they're
> committers in the first place. Certainly if a committer looks at the CF
> application and notices that 80% of the reviewing and committing is
> being done by three people, none of whom have any more "spare time" than
> they do, they should feel obligated to help those people out.

How many patches did you review? You don't have to be a committer to do that.

> We have a problem with patch reviewing and committing in this project;
> it's not being done in a timely fashion in general (every CF last year
> ended late), and the people who are doing most of the work feel
> overworked and frustrated. This problem is getting worse every year,
> and will kill the project if it continues on its current trajectory.

As if publicly blaming people for not behaving the way you would like them to
will do the project a lot of good.

Let me stress again that you didn't even try talking to the people in question
in private before, nor did you bother putting your *suggestion* up for
discussion before flaming people.

Also let me stress again that I did *not* put a patch into the CF.

> 3) getting most of our existing contributors to shoulder their fair
> share of patch review.
>
> (3) is what I'm addressing on this thread. The reason I volunteered to
> be CFM this time was directly because of our discussion in Ottawa of how
> the review process wasn't working. I decided to find out *why* it
> wasn't working, and the first obvious thing I ran across was that most
> of our current and our long-term contributors weren't doing any patch
> review. For CF1, the number of people submitting patches outnumbered
> those who had volunteered for review 2 to 1. That *is* the review
> problem in a nutshell; everybody wants someone else to do the work.

Great, I wasn't part of any discussion as I didn't make it to Ottawa this time.
Neither am I part of the problem with 0 patches, but still I've got to shoulder
the blame in a less than friendly way.

> I don't think it's too much to ask people who are listed on the project
> developers page as major contributors to review one patch per CommitFest
> most of the time. If they did just *one* it would substantially
> decrease the workload on the people who are currently doing the vast
> majority of review and commit.

You didn't ask! You blamed and offended people!

I won't go into details here because frankly why I have no time for reviewing a
patch is none of your business.

Michael

--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
Jabber: michael.meskes at gmail dot com
VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL

On Wed, Jul 03, 2013 at 04:03:08PM -0400, Bruce Momjian wrote:
> I do understand Josh's frustration that something different had to be
> done.

As a matter of fact I do, too. I just think the style of blaming people in
public like this is not ideal.

As I said I didn't even notice this email in the first hand until I was
approached from other people and called a slacker in communication not related
to the CF at all.

Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
Jabber: michael.meskes at gmail dot com
VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL

On 07/03/2013 02:03 PM, Michael Meskes wrote:
> I won't go into details here because frankly why I have no time for reviewing a
> patch is none of your business.

Then just send an email saying "Sorry, I don't have any time for patch
review this time. Maybe next time". It's pretty simple.

I'm not going to apologize for expecting *committers* to participate in
patch review and commit.

> As I said I didn't even notice this email in the first hand until I was
> approached from other people and called a slacker in communication not
related
> to the CF at all.

Ah, now, *that* wasn't my intent, sorry about that. It's rather a
surprise to me that anyone off of the -hackers list would care.

Possibly "slacker" was a poor choice of word given translations; in
colloquial American English it's a casual term, even affectionate under
some conditions. I'll make sure to use different words if I ever end up
doing a list again.

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

On 2013-07-03 14:16:09 -0700, Josh Berkus wrote:
> On 07/03/2013 02:03 PM, Michael Meskes wrote:
> > I won't go into details here because frankly why I have no time for reviewing a
> > patch is none of your business.
>
> Then just send an email saying "Sorry, I don't have any time for patch
> review this time. Maybe next time". It's pretty simple.
>
> I'm not going to apologize for expecting *committers* to participate in
> patch review and commit.

I find it absurd to expect anybody - including committers and regular
contributors - to be involved in the project all the time. It's one
thing to call somebody out who regularly commits his/her own stuff but
doesn't do CF work, something entirely different to do it to people who
didn't have time (or even chose to invest it differently!) to contribute
lately. The project does *NOT* own them.

I'd be at least as pissed as Michael seems to be if I were in his shoes.

Greetings,

Andres Freund

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

On Wed, Jul 3, 2013 at 5:16 PM, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
> I'm not going to apologize for expecting *committers* to participate in
> patch review and commit.

You are way out of line. You have no right to expect ANYONE to
participate in patch review and commit. Michael is doing us a favor
by maintaining ECPG even though he's not heavily involved in the
project any more and has other things to do with his time. I think
you're about two emails away from him having him resign in disgust,
and if he does, then the burden of reviewing and committing ECPG
patches is going to fall on someone else. Do you expect Tom or Noah
or Simon or myself to pick up the slack after you've driven him away?
I suppose you probably do, and that is absolutely wrong and really
pretty offensive.

I think it's completely appropriate for you to remind people who have
submitted patches for review but not reviewed any that they need to do
that part of it, too. Fair is fair. But you cannot enforce mandatory
volunteerism on people just because they are committers. Maybe you
think the world would be a better place if committers who didn't pull
their weight had their commit bits pulled, or that it doesn't matter
if you drive them to resign in disgust. I respectfully disagree.
Yeah, committers who are completely idle and never do anything
probably shouldn't have a commit bit. But someone like Michael who
reliably maintains ECPG is an asset to the project whether he chooses
to do anything else or not. I'm flabbergasted that you can't see
that.

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

On 07/03/2013 03:08 PM, Robert Haas wrote:
> You are way out of line. You have no right to expect ANYONE to
> participate in patch review and commit. Michael is doing us a favor
> by maintaining ECPG even though he's not heavily involved in the
> project any more and has other things to do with his time.

That's a good point. I hadn't considered (or realized the extent of)
the occasional and specific nature of Michael's involvement with the
project these days. My apologies, then, Michael.

Is there anyone else on the committer list with similar circumstances?

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

On Wed, Jul 3, 2013 at 6:34 PM, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
> On 07/03/2013 03:08 PM, Robert Haas wrote:
>> You are way out of line. You have no right to expect ANYONE to
>> participate in patch review and commit. Michael is doing us a favor
>> by maintaining ECPG even though he's not heavily involved in the
>> project any more and has other things to do with his time.
>
> That's a good point. I hadn't considered (or realized the extent of)
> the occasional and specific nature of Michael's involvement with the
> project these days. My apologies, then, Michael.
>
> Is there anyone else on the committer list with similar circumstances?

I would say that everyone on the committer list and every other list
has the right to choose the level of their involvement. We can't
really complain about what people choose to do or not do except to the
extent that they impose burdens on other people. For example, we
typically expect that if a committer commits a patch that breaks
something, that committer will promptly fix it. People who are not
willing to do that should not commit (or be allowed to commit). And
people who submit patches for review should also review patches: they
are asking other people to do work, so they should also contribute
work.

To the best of my knowledge, no one has ever been made a committer on
the condition that they spend a certain minimum number of hours per
week, month, or CommitFest on patch review, and I don't think we have
any right to expect that they do that. Rather, we should be thanking
the people who do choose to do more than their share of patch review,
whether they are committers or not. And the only people who need to
be called out are people who impose work on others without doing any
themselves. People who contribute a small amount but ask nothing for
it are a good thing, not a bad thing, again, regardless of whether
they have a commit bit.

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

On Wed, Jul 3, 2013 at 03:34:06PM -0700, Josh Berkus wrote:
> On 07/03/2013 03:08 PM, Robert Haas wrote:
> > You are way out of line. You have no right to expect ANYONE to
> > participate in patch review and commit. Michael is doing us a favor
> > by maintaining ECPG even though he's not heavily involved in the
> > project any more and has other things to do with his time.
>
> That's a good point. I hadn't considered (or realized the extent of)
> the occasional and specific nature of Michael's involvement with the
> project these days. My apologies, then, Michael.
>
> Is there anyone else on the committer list with similar circumstances?

I spend my available time going through old emails and finding issues
that never made it to a commit-fest, but need doing. I am currently in
November, 2012.

I am volunteering that information, and do not in any way feel I have to
justify my time commitment to anyone, except perhaps my employer. If
you want, you can remove my commit bit and I will just post all my
patches for others to commit --- hard to see how that is an improvement.
I will also remind you that before there were commit-fests, Tom and I
pretty much did all that work of committing non-committer's patches.

But my big feedback is, our community has no right to be asking about
committer circumstances. This is a voluntteer project, and people work
as they want. The extrapolation of Josh's approach is that committers
have to do work that the community wants to maintain their commit
rights, but their commit rights are helping the community, so why would
people care if you take them away --- you only hurt the community
further by doing so.

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

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

First of all, I'd like to give a big Thank You to all the hackers and
slackers that make Postgres great. You've really done an amazing job.

I'll step up and take a healthy portion of the blame here.  I enjoy the
awesome features & fixes that all of you put out year after year, but I
have yet to contribute anything.  For what it's worth, I'm sorry.  If
more guys like me could find some time to step up our game a little, we
wouldn't even be having this conversation.  But we're still left with
the fact that there is too much code and not enough review.

Honestly, there is a lot of work for committers to do even when all the
patches have been through sufficient code review.  Slogging through 4
months of CF patches without adequate review is enough to make anybody
throw up their hands and quit.

The best thing we can do to improve Postgres right now and over the long
term is to make sure that doesn't happen.  Anybody who is on this list
is a valued Postgres contributor, and that's why Josh has been reaching
out.  Maybe the term slacker offended some people, but he's basically
saying that code review is an essential contribution, perhaps more
essential than most new patches themselves at this point.  He's asking
for help from those of you with the skill, experience, and established
desire to contribute to the growth of Postgres.

Coordinating volunteers for anything is a frustrating process.  I'm sure
some people on the list shouldn't be on the list, and some people who
should be on the list aren't.  Maybe the list shouldn't have been sent
in the first place. But it's a call for help born out of frustration,
and one that we could remedy by stepping up our communication a bit.  5
days is too short?  How abut 7 or 10?  No time to review a patch this
CF?  Okay, good to know.

He's trying to tell us how we can best contribute.  If I manage to find
the time to contribute something over the next couple of weeks, it would
border on the absurd if my contribution was some sort of patch
submission.  Pretty much nothing I could write would be a more valuable
contribution than code review at this point.

So I'll drop Josh an email and let him know how much time I might be
able to contribute and when, and I'd suggest other people do the same.
Even if it's just a quick "Hey, I'm pretty busy the next couple weeks so
I'm not going to be able to review anything this CF."  Being armed with
a little more information about the potential volunteer pool would
probably make his job as CFM much easier.

Thanks again to all of you for all your hard work.

Best regards,

Wolfe Whalen

--
Wolfe Whalen
wolfe(at)quios(dot)net

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

Josh Berkus replied:
>> I won't go into details here because frankly why I have no time
>> for reviewing a patch is none of your business.
>
> Then just send an email saying "Sorry, I don't have any time for patch
> review this time. Maybe next time". It's pretty simple.

Hope about you not publically shame people in a volunteer project?
That's pretty simple.

> I'm not going to apologize for expecting *committers* to participate in
> patch review and commit.

I must have missed the page where patch review is defined as part of
a committer's job.

> Possibly "slacker" was a poor choice of word given translations; in
> colloquial American English it's a casual term, even affectionate under
> some conditions. I'll make sure to use different words if I ever end up
> doing a list again.

Please, don't ever do a list again. And yes, "slacker" was an extremely
poor choice of word. This American English speaker certainly has a
hard time viewing it as "affectionate". I think the whole thread would
have been better received with a subject line of "Commitfest needs help".

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

iEYEAREDAAYFAlHU1QQACgkQvJuQZxSWSsgoMgCfcUm/MnYzsUaqVWq3DvTh2kAi
sYwAoLAijh3SkCbv2c7visToyqPAOWMG
=xoKV
-----END PGP SIGNATURE-----

On 04/07/13 10:43, Robert Haas wrote:

> And
> people who submit patches for review should also review patches: they
> are asking other people to do work, so they should also contribute
> work.
>

I think that is an overly simplistic view of things. People submit
patches for a variety of reasons, but typically because they think the
patch will make the product better (bugfix or new functionality). This
is a contribution in itself, not a debt.

Now reviewing is performed to ensure that submitted code is *actually*
going to improve the product.

Both these activities are volunteer work - to attempt to tie them
together forceably is unusual to say the least. The skills and
experience necessary to review patches are considerably higher than
those required to produce patches, hence the topic of this thread.

Now I do understand we have a shortage of reviewers and lots of patches,
and that this *is* a problem - but what a wonderful problem...many open
source projects would love to be in this situation!!!

Regards

Mark

On Wed, Jul 3, 2013 at 03:34:06PM -0700, Josh Berkus wrote:
> On 07/03/2013 03:08 PM, Robert Haas wrote:
> > You are way out of line. You have no right to expect ANYONE to
> > participate in patch review and commit. Michael is doing us a favor
> > by maintaining ECPG even though he's not heavily involved in the
> > project any more and has other things to do with his time.
>
> That's a good point. I hadn't considered (or realized the extent of)
> the occasional and specific nature of Michael's involvement with the
> project these days. My apologies, then, Michael.
>
> Is there anyone else on the committer list with similar circumstances?

You know what this reminds me of --- early communist movements. Members
were scrutinized to see if they were working hard enough for "the
cause", and criticized/shamed/punished if they were not. The leaders
became tyrants, and justified the tyranny because they were creating "a
better society". We all know that didn't end well.

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

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

On 07/04/2013 09:09 AM, Bruce Momjian wrote:
> On Wed, Jul 3, 2013 at 03:34:06PM -0700, Josh Berkus wrote:
>> On 07/03/2013 03:08 PM, Robert Haas wrote:
>>> You are way out of line. You have no right to expect ANYONE to
>>> participate in patch review and commit. Michael is doing us a favor
>>> by maintaining ECPG even though he's not heavily involved in the
>>> project any more and has other things to do with his time.
>> That's a good point. I hadn't considered (or realized the extent of)
>> the occasional and specific nature of Michael's involvement with the
>> project these days. My apologies, then, Michael.
>>
>> Is there anyone else on the committer list with similar circumstances?
> You know what this reminds me of --- early communist movements. Members
> were scrutinized to see if they were working hard enough for "the
> cause", and criticized/shamed/punished if they were not. The leaders
> became tyrants, and justified the tyranny because they were creating "a
> better society". We all know that didn't end well.
>

I think that's way over the top. Can we all just cool down a bit? I
really don't see Josh as Stalin.

cheers

andrew (who came top of Russian History in his final year)

On Thu, Jul 4, 2013 at 09:16:22AM -0400, Andrew Dunstan wrote:
>
> On 07/04/2013 09:09 AM, Bruce Momjian wrote:
> >On Wed, Jul 3, 2013 at 03:34:06PM -0700, Josh Berkus wrote:
> >>On 07/03/2013 03:08 PM, Robert Haas wrote:
> >>>You are way out of line. You have no right to expect ANYONE to
> >>>participate in patch review and commit. Michael is doing us a favor
> >>>by maintaining ECPG even though he's not heavily involved in the
> >>>project any more and has other things to do with his time.
> >>That's a good point. I hadn't considered (or realized the extent of)
> >>the occasional and specific nature of Michael's involvement with the
> >>project these days. My apologies, then, Michael.
> >>
> >>Is there anyone else on the committer list with similar circumstances?
> >You know what this reminds me of --- early communist movements. Members
> >were scrutinized to see if they were working hard enough for "the
> >cause", and criticized/shamed/punished if they were not. The leaders
> >became tyrants, and justified the tyranny because they were creating "a
> >better society". We all know that didn't end well.
> >
>
> I think that's way over the top. Can we all just cool down a bit? I
> really don't see Josh as Stalin.

I don't either. It is the "judging others efforts" that concerns me.

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

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

On Thu, Jul 04, 2013 at 08:08:57PM +1200, Mark Kirkwood wrote:
> On 04/07/13 10:43, Robert Haas wrote:
>
>> And
>> people who submit patches for review should also review patches: they
>> are asking other people to do work, so they should also contribute
>> work.
>>
>
> I think that is an overly simplistic view of things. People submit
> patches for a variety of reasons, but typically because they think the
> patch will make the product better (bugfix or new functionality). This
> is a contribution in itself, not a debt.

True. I don't see that policy as a judgment against the value of submissions,
but rather a response ...

> Now reviewing is performed to ensure that submitted code is *actually*
> going to improve the product.
>
> Both these activities are volunteer work - to attempt to tie them
> together forceably is unusual to say the least. The skills and
> experience necessary to review patches are considerably higher than
> those required to produce patches, hence the topic of this thread.
>
> Now I do understand we have a shortage of reviewers and lots of patches,

... to this. Reviewing may be harder than writing a patch, but patch
submitters are more promising as reviewers than any other demographic. The
situation has a lot in common with the system of academic peer review:
http://en.wikipedia.org/wiki/Peer_review#Scholarly_peer_review

It's a good value for submitters. By the time my contributions are part of a
release, they've regularly become better than I would have achieved working in
isolation. Reviewers did that.

> and that this *is* a problem - but what a wonderful problem...many open
> source projects would love to be in this situation!!!

A database is different from much other software in that users build
intricate, long-lived software of their own against it. In that respect, it's
like the hardware-independent part of a compiler or an OS kernel. When we
establish an interface, we maintain it forever or remove it at great user
cost. It's also different by virtue of managing long-term state, like a
filesystem. That dramatically elevates the potential cost of a bug. A
spreadsheet program might reasonably have a different perspective on a surge
of submissions. For PostgreSQL, figuring out how to review them is key.

--
Noah Misch
EnterpriseDB http://www.enterprisedb.com

All,

>> I think that's way over the top. Can we all just cool down a bit? I
>> really don't see Josh as Stalin.
>
> I don't either. It is the "judging others efforts" that concerns me.
>

I agree that publishing the committer portion of the list was a mistake,
and will not include it in the future CFM instructions.

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

On Thu, Jul 4, 2013 at 6:16 AM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>> You know what this reminds me of --- early communist movements. Members
>> were scrutinized to see if they were working hard enough for "the
>> cause", and criticized/shamed/punished if they were not. The leaders
>> became tyrants, and justified the tyranny because they were creating "a
>> better society". We all know that didn't end well.
>>
>
> I think that's way over the top. Can we all just cool down a bit? I really
> don't see Josh as Stalin.

+1. I think that Josh misjudged things in starting this thread, but
there is no need for this kind of rhetoric. It helps no one.

--
Peter Geoghegan

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

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

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

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

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

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

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

Cheers,

Jeff

On 07/05/2013 02:34 PM, Jeff Janes wrote:
> On Wed, Jul 3, 2013 at 12:03 PM, Christopher Browne <cbbrowne(at)gmail(dot)com>wrote:
>
>> On Wed, Jul 3, 2013 at 2:24 PM, Cédric Villemain <cedric(at)2ndquadrant(dot)com>wrote:
>>> Others rules appeared, like the 5 days limit.
>>>
>>
> The limit was previously 4 days (at least according to
> http://wiki.postgresql.org/wiki/RRReviewers) but I think that that was
> honored almost exclusively in the breach. I don't have a sense for how the
> 5 day limit is working. If it is working, great. If not, I would advocate
> lengthening it--having a limit specified but not generally held to is
> counterproductive. I know that I, and at least one other potential
> reviewer, won't ask to be assigned a random patch because we have no
> confidence we can do an adequate review of a random patch within a
> contiguous 5 day window.

Well, keep in mind that the "reviewer" blank on the commitfest page is
solely a reservation to prevent duplicate effort. There is absolutely
nothing preventing someone from doing and submitting a review without
putting their name down -- and several have, this CF.

The idea of the 5-day limit is that this often happens:
1. reviewer takes patch because they want to review it
2. something comes up in their life outside the postgres project, and
they don't get time to review it
3. because their name is on the patch as "reviewer", others don't pick
it up for review
4. it gets to 3 days before CF end, and nobody has reviewed the patch

While there are certainly cases where a reviewer takes more than 5 days
actively to review a patch, these are the minority. Most of the time, a
reviewer who doesn't review a patch within 5 days doesn't get around to
reviewing it that commitfest, period.

If you are actively reviewing for more than 5 days, simply send me/Mike
an email saying so, before the 5 days are up. Or post something on
-hackers in the patch thread.

Previously, we dealt with this by sending out an "are you reviewing this
patch" email from the CFM to the reviewer after 5 days passed. The
problem with that is that a reviewer who was too busy to review was
usually too busy to answer, and as a result the whole cycle of "he's not
gonna review that" took around 10 days on average before we knew we
could remove the name. 10 days is 1/3 of the total commitfest --
unacceptably long. Thus the approach we're trying this CF.

Again, this is an area where better automation could really help.
Reviewers could get automated reminders at 4 and 5 days, and ack those
reminders to extend the review period.

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

Please do!

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

Unfortunately, the only time we guarantee that a patch or even a spec
proposal will get a hearing and discussion is the CF. Thus people who
really want to get agreement on a prototype, spec, proposal or API are
gonna submit it to the CF, so that they get some useful feedback. Most
of the time, someone posting a WIP, API, spec, SQL syntax or feature
concept outside of a CF gets little or no useful criticism/suggestions
on it.

If we don't want WIP/RFC patches in the CF, then we need to provide some
other way to guarantee that these incomplete patches will get feedback.
I'd be in favor of having something -- I think more authors would get
better feedback early on in development -- but I have no idea what it
would be.

Other uncommittable patches submitted to the CF are there because the
submitter is sending in a first-time patch. It's very important for
training up the new submitter that their patch get the full
review-returned-with-feedback cycle. That's how they become better
patch authors in the future. Personally, I think this is one of the
most valuable aspects of the CF process.

>> Well, if the project is hampered by not being able to get *all* the
>> changes that people imagine that they want to put in, then we have a
>> real problem of needing a sort of "triage" to determine which changes
>> will be accepted, and which will not.
>>
>> Perhaps we need an extra status in the CommitFest application, namely
>> one that characterizes:
>> Insufficiently Important To Warrant Review

Gods forbid. We might as well have the tag "Stupid Idiot Patch" and be
done with it. And people accuse *me* of being submitter-hostile.

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

Exactly.

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

* Josh Berkus (josh(at)agliodbs(dot)com) wrote:
> Is there anyone else on the committer list with similar circumstances?

I'll just flip it around and offer to be publically flogged whenever I'm
not helping out with a commitfest. :) Perhaps this should be more
"opt-in" than "opt-out", wrt committers anyway.

Thanks,

Stephen

----- Original Message -----
> * Josh Berkus (josh(at)agliodbs(dot)com) wrote:
> > Is there anyone else on the committer list with similar circumstances?
>
> I'll just flip it around and offer to be publically flogged whenever I'm
> not helping out with a commitfest. :) Perhaps this should be more
> "opt-in" than "opt-out", wrt committers anyway.

Can we flog you even if you *are* helping? I just wanna see the YouTube video, either way. ;-)

On 6/24/13 12:57 PM, Josh Berkus wrote:
> Maciej is correct that this policy also belongs on the "how to submit a
> patch" wiki page. I will remedy that.

I just reviewed and heavily updated the new section you added to
https://wiki.postgresql.org/wiki/Submitting_a_Patch That included the
idea that the reviewed patch should be similar in size/scope to the
submitted one, as well a slightly deeper discussion of how to fit this
work into a feature review quote.

I find myself needing to explain this whole subject to potential feature
sponsors enough that I've tried a few ways of describing it. The
closest analog I've found so far is the way "carbon offset" work is
accounted for. I sometimes refer to the mutual review as an "offsetting
review" in conversation, and I threw that term into the guidelines as well.

As far as motivating new reviewers goes, let's talk about positive
feedback. Anything that complicates the release notes is a non-starter
because that resource is tightly controlled by a small number of people,
and it's trying to satisfy a lot of purposes. What I would like to see
is an official but simple "Review Leaderboard" for each release instead.

Each time someone writes a review and adds it to CF app with a "Review"
entry, the account that entered it gets a point. Sum the points at the
end and there's your weighted list for T-shirt prizes. It should be
possible to get that count with a trivial SELECT query out of the CF
database, and then produce a simple HTML table at the end of each CF or
release. Anything that takes more work than that, and anything that
takes *any* time that must come from a committer, is too much accounting.

This idea would be a bit unfair to people who review big patches instead
of little ones. But an approach that disproportionately rewards new
contributors working on small things isn't that terrible. As long as
the review tests whether the code compiles and passes the regression
tests, that's good enough to deserve a point. I'd be happy if we
suddenly had a problem where people were doing only that to try game
their leaderboard ranking.

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

On 7/3/13 7:25 PM, Bruce Momjian wrote:
> The extrapolation of Josh's approach is that committers
> have to do work that the community wants to maintain their commit
> rights, but their commit rights are helping the community, so why would
> people care if you take them away --- you only hurt the community
> further by doing so.

The main problem with having inactive committers (which I don't intend
to include the important subject matter committers, who I'll get into at
the end here) is that they skew the public information about who commits
in a counterproductive way. People visit
https://wiki.postgresql.org/wiki/Committers , sees that list of names,
and then make conclusions based on its content. And some of those
conclusions are wrong because the data is inconsistent. The standards
listed are applied when new committers are promoted, but they are not
applied symmetrically to remove ones who don't anymore.

The #1 obstacle to my getting more time to work on core PostgreSQL is
that companies presume regular submitters who are also non-committers
don't do a very good job. If you are competent and have a consistent
track record of contributions to an open source project, the project
would make you a committer, right? Conversely, if you've been
contributing for a while but aren't a committer, the most likely
explanation is that your work quality is poor. That is a completely
reasonable viewpoint based on how most open source projects work. The
really terrible part is that it means the longer you've been submitting
patches, the *less* competent you're assumed to be. When I tell people
I've been submitting things since 2007 but am not a committer, the only
logical explanation is that my submissions must suck very hard, right?

From that perspective, people who are listed as committers but don't
actively do work for the project are causing me a serious problem. When
someone who rarely commits can obviously qualify, that *proves* the bar
for PostgreSQL committers is actually very low to casual observers.
That's the message the project is inadvertently sending by leaving
committers on there if they stop working actively.

The main thing I'd like to see is having the committer list, and its
associated guidelines, updated to reflect that there are subject matter
experts committing too. That would pull them out of any "what have you
done for me lately?" computations, and possibly open up a way to get
more of them usefully. Here are the first two obvious labels like that:

Michael Meskes (meskes) - embedded SQL
Teodor Sigaev (teodor) - inverted indexes

When even Josh Berkus doesn't even know all of this information, it's
clearly way too obscure to expect the rest of the world to figure it out.

It also boggles my mind that there isn't already an entry like this on
there too:

Thom Browne - documentation

Each time Thom passes through yet another correction patch that is
committed with no change, I find it downright bizarre that a community
with such limited committer resources wastes their time with that
gatekeeping. The standards for nominating committers seem based on
whether they can commit just about anything. I think it's more
important to consider whether people are trusted to keep commits within
their known area(s) of expertise.

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

Greg,

> As far as motivating new reviewers goes, let's talk about positive
> feedback. Anything that complicates the release notes is a non-starter
> because that resource is tightly controlled by a small number of people,
> and it's trying to satisfy a lot of purposes.

Greg, you're re-arguing something we obtained a consensus on a week ago.
Please read the thread "Kudos for reviewers".

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