FAQ
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

Search Discussions

  • Andrew Dunstan at Jun 24, 2013 at 12:14 pm

    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
  • Dimitri Fontaine at Jun 24, 2013 at 3:01 pm

    Andrew Dunstan 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
  • Maciej Gajewski at Jun 24, 2013 at 3:40 pm
    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
  • Andreas Karlsson at Jun 24, 2013 at 3:43 pm

    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
  • Joshua D. Drake at Jun 24, 2013 at 3:55 pm

    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
  • Dimitri Fontaine at Jun 24, 2013 at 4:07 pm

    "Joshua D. Drake" <jd@commandprompt.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
  • Josh Berkus at Jun 24, 2013 at 5:22 pm

    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
  • Claudio Freire at Jun 24, 2013 at 5:30 pm

    On Mon, Jun 24, 2013 at 2:22 PM, Josh Berkus 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.
  • Joshua D. Drake at Jun 24, 2013 at 5:37 pm

    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
  • Andres Freund at Jun 24, 2013 at 5:40 pm

    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
  • Claudio Freire at Jun 24, 2013 at 5:48 pm

    On Mon, Jun 24, 2013 at 2:41 PM, Andres Freund 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.
  • Joshua D. Drake at Jun 24, 2013 at 5:53 pm

    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
  • Andres Freund at Jun 24, 2013 at 5:55 pm

    On 2013-06-24 14:48:32 -0300, Claudio Freire wrote:
    On Mon, Jun 24, 2013 at 2:41 PM, Andres Freund 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
  • Josh Berkus at Jun 24, 2013 at 5:57 pm

    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
  • Josh Berkus at Jun 24, 2013 at 5:50 pm

    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
  • Andres Freund at Jun 24, 2013 at 5:58 pm

    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
  • Joshua D. Drake at Jun 24, 2013 at 6:13 pm

    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
  • Brendan Jurd at Jun 25, 2013 at 8:21 am

    On 25 June 2013 04:13, Joshua D. Drake 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
  • Hannu Krosing at Jun 24, 2013 at 4:15 pm

    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Ü
  • Mark Kirkwood at Jun 24, 2013 at 10:24 pm

    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
  • Tom Lane at Jun 25, 2013 at 4:02 am

    Mark Kirkwood 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
  • Mark Kirkwood at Jun 25, 2013 at 6:20 am

    On 25/06/13 15:56, Tom Lane wrote:
    Mark Kirkwood <mark.kirkwood@catalyst.net.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
  • David Rowley at Jun 25, 2013 at 7:14 am

    On Tue, Jun 25, 2013 at 3:56 PM, Tom Lane 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
  • Josh Berkus at Jun 24, 2013 at 4:57 pm

    On 06/24/2013 08:01 AM, Dimitri Fontaine wrote:
    Andrew Dunstan <andrew@dunslane.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
  • Dimitri Fontaine at Jun 24, 2013 at 5:02 pm

    Josh Berkus 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
  • Josh Berkus at Jun 24, 2013 at 5:10 pm

    On 06/24/2013 10:02 AM, Dimitri Fontaine wrote:
    Josh Berkus <josh@agliodbs.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
  • Andres Freund at Jun 24, 2013 at 5:19 pm

    On 2013-06-24 10:10:11 -0700, Josh Berkus wrote:
    On 06/24/2013 10:02 AM, Dimitri Fontaine wrote:
    Josh Berkus <josh@agliodbs.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
  • Josh Berkus at Jun 24, 2013 at 5:25 pm

    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
  • Atri Sharma at Jun 24, 2013 at 5:29 pm

    On Mon, Jun 24, 2013 at 10:54 PM, Josh Berkus 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
  • Joshua D. Drake at Jun 24, 2013 at 5:24 pm

    On 06/24/2013 10:10 AM, Josh Berkus wrote:
    On 06/24/2013 10:02 AM, Dimitri Fontaine wrote:
    Josh Berkus <josh@agliodbs.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
  • Josh Berkus at Jun 24, 2013 at 5:40 pm

    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
  • Bruce Momjian at Jun 24, 2013 at 7:14 pm

    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@momjian.us> http://momjian.us
       EnterpriseDB http://enterprisedb.com

       + It's impossible for everything to be true. +
  • Robert Haas at Jun 24, 2013 at 6:17 pm

    On Mon, Jun 24, 2013 at 1:24 PM, Joshua D. Drake 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
  • Simon Riggs at Jun 24, 2013 at 5:46 pm

    On 24 June 2013 18:10, Josh Berkus 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
  • Noah Misch at Jun 25, 2013 at 2:07 am

    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@agliodbs.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
  • Michael Paquier at Jun 25, 2013 at 2:11 am

    On Tue, Jun 25, 2013 at 11:06 AM, Noah Misch 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@agliodbs.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
  • Josh Kupershmidt at Jun 24, 2013 at 5:44 pm

    On Mon, Jun 24, 2013 at 12:57 PM, Josh Berkus 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
  • Josh Berkus at Jun 24, 2013 at 5:47 pm

    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
  • Szymon Guz at Jun 24, 2013 at 6:48 pm
    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
  • Josh Berkus at Jun 24, 2013 at 7:00 pm
    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
  • Greg Smith at Jul 22, 2013 at 11:16 pm

    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@2ndquadrant.com Baltimore, MD
    PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.com
  • Josh Berkus at Jul 23, 2013 at 4:20 pm
    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
  • Robins at Jun 26, 2013 at 7:23 am
    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 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@postgresql.org)
    To make changes to your subscription:
    http://www.postgresql.org/mailpref/pgsql-hackers
  • Michael Meskes at Jul 2, 2013 at 8:52 am
    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
  • Bruce Momjian at Jul 2, 2013 at 2:44 pm

    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@momjian.us> http://momjian.us
       EnterpriseDB http://enterprisedb.com

       + It's impossible for everything to be true. +
  • Michael Paquier at Jul 2, 2013 at 9:40 pm

    On 2013/07/02, at 23:44, Bruce Momjian 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
  • Josh Berkus at Jul 3, 2013 at 4:42 am
    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
  • Michael Meskes at Jul 3, 2013 at 1:29 pm

    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
  • Josh Berkus at Jul 3, 2013 at 7:34 pm

    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
  • Bruce Momjian at Jul 3, 2013 at 8:03 pm

    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@momjian.us> http://momjian.us
       EnterpriseDB http://enterprisedb.com

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

Related Discussions

People

Translate

site design / logo © 2021 Grokbase