Thread: Kudos for Reviewers -- straw poll
Hackers, I'd like to take a straw poll here on how we should acknowledge reviewers. Please answer the below with your thoughts, either on-list or via private email. How should reviewers get credited in the release notes? a) not at all b) in a single block titled "Reviewers for this version" at the bottom. c) on the patch they reviewed, for each patch Should there be a criteria for a "creditable" review? a) no, all reviews are worthwhile b) yes, they have to do more than "it compiles" c) yes, only code reviews should count Should reviewers for 9.4 get a "prize", such as a t-shirt, as a promotion to increase the number of non-submitter reviewers? a) yes b) no c) yes, but submitters and committers should get it too Thanks for your feedback! -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
On 25.06.2013 20:17, Josh Berkus wrote: > Hackers, > > I'd like to take a straw poll here on how we should acknowledge > reviewers. Please answer the below with your thoughts, either on-list > or via private email. > > How should reviewers get credited in the release notes? > > a) not at all > b) in a single block titled "Reviewers for this version" at the bottom. > c) on the patch they reviewed, for each patch a) Sometimes a reviewer contributes greatly to the patch, revising it and rewriting parts of it. At that point, it's not just a review anymore, and he/she should be mentioned in the release notes as a co-author. > Should there be a criteria for a "creditable" review? > > a) no, all reviews are worthwhile > b) yes, they have to do more than "it compiles" > c) yes, only code reviews should count This is one reason why I answered a) above. I don't want to set a criteria. > Should reviewers for 9.4 get a "prize", such as a t-shirt, as a > promotion to increase the number of non-submitter reviewers? > > a) yes > b) no > c) yes, but submitters and committers should get it too a). I don't think we should make any promises, though. Just arbitrarily send a t-shirt when you feel that someone has done a good job reviewing other people's patches. And I'm not sure it's really worth the trouble, to arrange the logistics etc. - Heikki
On Tue, June 25, 2013 19:17, Josh Berkus wrote: > How should reviewers get credited in the release notes? b) in a single block titled "Reviewers for this version" at the bottom. > Should there be a criteria for a "creditable" review? b) yes, they have to do more than "it compiles" > Should reviewers for 9.4 get a "prize", such as a t-shirt, as a > promotion to increase the number of non-submitter reviewers? b) no Erik Rijkers
On 2013-06-25 10:17:07 -0700, Josh Berkus wrote: > How should reviewers get credited in the release notes? > > a) not at all > b) in a single block titled "Reviewers for this version" at the bottom. > c) on the patch they reviewed, for each patch b). If the review was substantial enough the reviewer gets bumped to a secondary author, just as it already happens. > Should there be a criteria for a "creditable" review? > > a) no, all reviews are worthwhile > b) yes, they have to do more than "it compiles" > c) yes, only code reviews should count b). Surely performance reviews should also count, they can be at least as time consuming as a code review, so c) doesn't seem to make sense. > Should reviewers for 9.4 get a "prize", such as a t-shirt, as a > promotion to increase the number of non-submitter reviewers? > > a) yes > b) no > c) yes, but submitters and committers should get it too Not sure. Seems like it might be a way to spend a lot of effort without achieving all that much. But I can also imagine that it feels nice and encourages a casual reviewer/contributor. So it's either b) or c). Although I'd perhaps exclude regular contributors to keep the list reasonable? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On 06/25/2013 10:46 AM, Andres Freund wrote: > Not sure. Seems like it might be a way to spend a lot of effort without > achieving all that much. But I can also imagine that it feels nice and > encourages a casual reviewer/contributor. > > So it's either b) or c). Although I'd perhaps exclude regular > contributors to keep the list reasonable? Well, one of the other "prizes" which occurred to me today would be a pgCon lottery. That is, each review posted by a non-committer would go in a hat, and in February we would draw one who would get a free registration and airfare to pgCon. Seems apropos and without the horrible logistics issues of mailing tshirts to 15 countries. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
On 06/25/2013 10:17 AM, Josh Berkus wrote: > > Hackers, > > I'd like to take a straw poll here on how we should acknowledge > reviewers. Please answer the below with your thoughts, either on-list > or via private email. > > How should reviewers get credited in the release notes? > > a) not at all > b) in a single block titled "Reviewers for this version" at the bottom. > c) on the patch they reviewed, for each patch C. The idea that reviewers are somehow less than authors is rather disheartening. > > Should there be a criteria for a "creditable" review? > > a) no, all reviews are worthwhile > b) yes, they have to do more than "it compiles" > c) yes, only code reviews should count B. I think it compiles, and I tested it via X should be the minimum. Here is a case. I was considering taking a review of the new Gin Cache patch. I can't really do a "code" review but I can certainly run benchmarking tests before/after and apply the patch etc. > > Should reviewers for 9.4 get a "prize", such as a t-shirt, as a > promotion to increase the number of non-submitter reviewers? > > a) yes > b) no > c) yes, but submitters and committers should get it too > > Thanks for your feedback! > B. We already give them a multi-million dollar piece of software for free. JD -- Command Prompt, Inc. - http://www.commandprompt.com/ 509-416-6579 PostgreSQL Support, Training, Professional Services and Development High Availability, Oracle Conversion, Postgres-XC, @cmdpromptinc For my dreams of your image that blossoms a rose in the deeps of my heart. - W.B. Yeats
On 2013-06-25 11:04:38 -0700, Joshua D. Drake wrote: > >a) not at all > >b) in a single block titled "Reviewers for this version" at the bottom. > >c) on the patch they reviewed, for each patch > > C. The idea that reviewers are somehow less than authors is rather > disheartening. It's not about the reviewers being less. It's a comparison of effort. The effort for a casual review simply isn't comparable with the effort spent on developing a nontrivial patch. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On 06/25/2013 01:17 PM, Josh Berkus wrote: > Hackers, > > I'd like to take a straw poll here on how we should acknowledge > reviewers. Please answer the below with your thoughts, either on-list > or via private email. > > How should reviewers get credited in the release notes? > > a) not at all > b) in a single block titled "Reviewers for this version" at the bottom. > c) on the patch they reviewed, for each patch b) seems about right. > > Should there be a criteria for a "creditable" review? > > a) no, all reviews are worthwhile > b) yes, they have to do more than "it compiles" > c) yes, only code reviews should count c). Compilation, functionality and performance tests are useful, but what we desperately need are in depth code reviews of large patches. If we debase the currency by rewarding things less than that then any incentive effect of kudos in encouraging more reviews is lost. > > Should reviewers for 9.4 get a "prize", such as a t-shirt, as a > promotion to increase the number of non-submitter reviewers? > > a) yes > b) no > c) yes, but submitters and committers should get it too I'd like to see prizes each release for "best contribution" and "best reviewer" - I've thought for years something like this would be worth trying. Committers and core members should not be eligible - this is about encouraging new people. cheers andrew
On 06/25/2013 11:26 AM, Andres Freund wrote: > > On 2013-06-25 11:04:38 -0700, Joshua D. Drake wrote: >>> a) not at all >>> b) in a single block titled "Reviewers for this version" at the bottom. >>> c) on the patch they reviewed, for each patch >> >> C. The idea that reviewers are somehow less than authors is rather >> disheartening. > > It's not about the reviewers being less. It's a comparison of > effort. The effort for a casual review simply isn't comparable with the > effort spent on developing a nontrivial patch. I think this is a backwards way to look at it. The effort may not be comparable but the drudgery certainly is. Reviewing patches sucks. Writing patches (for the most part) is fun. Should the patch submitter get first billing? Yes. Should the reviewer that makes sure to a reasonable level that the patch is sane also get billing? Absolutely. Should the reviewer get billing that is about the patch they reviewed. Yes. As I mentioned before in the release notes something like: Author: Tom Lane Reviewer(s): Greg Stark, Andrew Dunstan I think that is perfectly reasonable. JD -- Command Prompt, Inc. - http://www.commandprompt.com/ 509-416-6579 PostgreSQL Support, Training, Professional Services and Development High Availability, Oracle Conversion, Postgres-XC, @cmdpromptinc For my dreams of your image that blossoms a rose in the deeps of my heart. - W.B. Yeats
On Tue, Jun 25, 2013 at 2:17 PM, Josh Berkus <josh@agliodbs.com> wrote: > How should reviewers get credited in the release notes? > c) on the patch they reviewed, for each patch This not only makes sense, it also lets people reading release notes know there's been a review, and how thorough it was. I know, all changes in PG get reviewed, but having it explicit on release notes I imagine would be useful. Especially if I know the reviewers. The co-author part also makes a lot of sense. When a reviewer introduces changes directly, and they get committed, I think it should be automatically considered co-authoring the patch. > Should there be a criteria for a "creditable" review? > > a) no, all reviews are worthwhile It's not that they're all worthwhile, but arbitrary decisions lend themselves to arbitrary criticism. Whatever criteria should be straightforward, unambiguous and unbiased, and it's hard getting all those three in any other criteria than: all are worthwhile. > b) yes, they have to do more than "it compiles" This one's better than nothing, if you must have a minimum criteria. But then people will just point out some trivial stuff and you'd be tempted to say that trivialities don't count... and you get a snowball going. IMHO, it's all or nothing. > Should reviewers for 9.4 get a "prize", such as a t-shirt, as a > promotion to increase the number of non-submitter reviewers? > > b) no Yeah, while a fun idea, I don't think the logistics of it make it worth the effort. Too much effort for too little benefit. And I think recognition is a far better incentive than T-shirts anyway. I know I'd be encouraged to review patches for the recognition alone, a lot more than for a T-shirt I might not get. Contributing is nice, but feeling appreciated while doing so is better.
On 06/25/2013 11:26 AM, Andres Freund wrote: > On 2013-06-25 11:04:38 -0700, Joshua D. Drake wrote: >>> a) not at all >>> b) in a single block titled "Reviewers for this version" at the bottom. >>> c) on the patch they reviewed, for each patch >> >> C. The idea that reviewers are somehow less than authors is rather >> disheartening. > > It's not about the reviewers being less. It's a comparison of > effort. The effort for a casual review simply isn't comparable with the > effort spent on developing a nontrivial patch. On the other hand, I will point out that we currently have a shortage of reviewers, and we do NOT have a shortage of patch submitters. Seems like we should apply incentives where we need help, no? Mind you, my votes are (B), (A), and (A). -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
On 25 June 2013 18:17, Josh Berkus <josh@agliodbs.com> wrote: > Hackers, > > I'd like to take a straw poll here on how we should acknowledge > reviewers. Please answer the below with your thoughts, either on-list > or via private email. > > How should reviewers get credited in the release notes? > > a) not at all > b) in a single block titled "Reviewers for this version" at the bottom. > c) on the patch they reviewed, for each patch > b) Unless they contribute enough to the patch to be considered a co-author. > Should there be a criteria for a "creditable" review? > > a) no, all reviews are worthwhile > b) yes, they have to do more than "it compiles" > c) yes, only code reviews should count > a) Sometimes even "it compiles" can be worthwhile, if there is doubt over platform compatibility. All contributions should be acknowledged and encouraged. > Should reviewers for 9.4 get a "prize", such as a t-shirt, as a > promotion to increase the number of non-submitter reviewers? > > a) yes > b) no > c) yes, but submitters and committers should get it too > b) Getting your name in the fine manual is reward enough ;-) Regards, Dean
On Tue, Jun 25, 2013 at 10:17:07AM -0700, Josh Berkus wrote: > Hackers, > > I'd like to take a straw poll here on how we should acknowledge > reviewers. Please answer the below with your thoughts, either on-list > or via private email. > > How should reviewers get credited in the release notes? > > a) not at all > b) in a single block titled "Reviewers for this version" at the bottom. > c) on the patch they reviewed, for each patch c) This keeps history better. > Should there be a criteria for a "creditable" review? > > a) no, all reviews are worthwhile > b) yes, they have to do more than "it compiles" > c) yes, only code reviews should count a). If it turns out that people are gaming this, or appear to be gaming this, we can revisit the policy. > Should reviewers for 9.4 get a "prize", such as a t-shirt, as a > promotion to increase the number of non-submitter reviewers? > > a) yes > b) no > c) yes, but submitters and committers should get it too b). You want to be *extremely* careful when paying volunteers. The chances of damaging their intrinsic motivations are high, especially when it's not stuff like covering their expenses. http://www.iew.uzh.ch/wp/iewwp007.pdf Cheers, David. -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
On 26 June 2013 03:17, Josh Berkus <josh@agliodbs.com> wrote: > How should reviewers get credited in the release notes? > > a) not at all > b) in a single block titled "Reviewers for this version" at the bottom. > c) on the patch they reviewed, for each patch A weak preference for (c), with (b) running a close second. As others have suggested, a review that leads to significant commitable changes to the patch should bump the credit to co-authorship. > Should there be a criteria for a "creditable" review? > > a) no, all reviews are worthwhile > b) yes, they have to do more than "it compiles" > c) yes, only code reviews should count (b), the review should at least look at usabililty, doc, and regression test criteria even if there is no in-depth code analysis. > Should reviewers for 9.4 get a "prize", such as a t-shirt, as a > promotion to increase the number of non-submitter reviewers? > > a) yes > b) no > c) yes, but submitters and committers should get it too Provisionally (b), if we first try giving proper credit, and that still doesn't drum up enough reviewing, then look to further incentive schemes. No need to jump the gun. Cheers, BJ
Brendan Jurd wrote > On 26 June 2013 03:17, Josh Berkus < > josh@ > > wrote: >> How should reviewers get credited in the release notes? >> >> a) not at all >> b) in a single block titled "Reviewers for this version" at the bottom. >> c) on the patch they reviewed, for each patch I think some consideration toward a "commit and review summary" (outside the release notes; and graphical/interactive in nature ideally) for each major release is something worth considering. With regards to the release notes I'd lean toward (b); significant contributions getting bumped to co-author on specific patches covers (c) fairly well. I am unsure whether release note mentions are significant enough motivation...see other thoughts below. >> Should there be a criteria for a "creditable" review? >> >> a) no, all reviews are worthwhile >> b) yes, they have to do more than "it compiles" >> c) yes, only code reviews should count Ideally (a) though (b) conceptually makes sense but it is too generic. >> Should reviewers for 9.4 get a "prize", such as a t-shirt, as a >> promotion to increase the number of non-submitter reviewers? >> >> a) yes >> b) no >> c) yes, but submitters and committers should get it too One low-cost "prize" that I've pondered is, on an ongoing basis, the ability to post a link and/or message to the PostgreSQL front page within a significantly less stringent barrier to "acceptance" than is required for current content. Basically except for topics or presentations deemed of poor taste or detrimental to the project anything should be allowed. Some kind of "this message was allowed because so-and-so has recently made the following significant contributions to the project". There are probably quite a few logistics to deal with down this path but a sponsor platform for shameless self-promotion for people making the project successful - something visible on an ongoing basis and not just once a year in a release note - is likely a very valuable to the contributor while fairly inexpensive to the project (i.e., some risk of reputation and some cost to setup the infrastructure). David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/Kudos-for-Reviewers-straw-poll-tp5760952p5761031.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.
Dean Rasheed wrote: >> How should reviewers get credited in the release notes? >> >> a) not at all >> b) in a single block titled "Reviewers for this version" at the bottom. >> c) on the patch they reviewed, for each patch >> > > b) Unless they contribute enough to the patch to be considered a co-author. > > >> Should there be a criteria for a "creditable" review? >> >> a) no, all reviews are worthwhile >> b) yes, they have to do more than "it compiles" >> c) yes, only code reviews should count >> > > a) Sometimes even "it compiles" can be worthwhile, if there is doubt > over platform compatibility. All contributions should be acknowledged > and encouraged. > > >> Should reviewers for 9.4 get a "prize", such as a t-shirt, as a >> promotion to increase the number of non-submitter reviewers? >> >> a) yes >> b) no >> c) yes, but submitters and committers should get it too >> > > b) Getting your name in the fine manual is reward enough ;-) +1, except that I like Josh's idea about a free ticket to pgCon. Yours, Laurenz Albe
For me, B,B and another B works. Regards, Atri -- Regards, Atri l'apprenant
On Wed, Jun 26, 2013 at 10:40:17AM +1000, Brendan Jurd wrote: > On 26 June 2013 03:17, Josh Berkus <josh@agliodbs.com> wrote: > > How should reviewers get credited in the release notes? > > > > a) not at all > > b) in a single block titled "Reviewers for this version" at the bottom. > > c) on the patch they reviewed, for each patch > > A weak preference for (c), with (b) running a close second. As others > have suggested, a review that leads to significant commitable changes > to the patch should bump the credit to co-authorship. As a reminder, I tried a variant of C for 9.2 beta release notes, and got lots of complaints, particularly because the line describing the feature now had many more names on it. In my opinion, adding reviewer names to each feature item might result in the removal of all names from features. A poll is nice for gauging interest, but many people who vote don't understand the ramifications of what they are voting on. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On 06/26/2013 09:14 AM, Bruce Momjian wrote: > On Wed, Jun 26, 2013 at 10:40:17AM +1000, Brendan Jurd wrote: >> On 26 June 2013 03:17, Josh Berkus <josh@agliodbs.com> wrote: >>> How should reviewers get credited in the release notes? >>> >>> a) not at all >>> b) in a single block titled "Reviewers for this version" at the bottom. >>> c) on the patch they reviewed, for each patch >> A weak preference for (c), with (b) running a close second. As others >> have suggested, a review that leads to significant commitable changes >> to the patch should bump the credit to co-authorship. > As a reminder, I tried a variant of C for 9.2 beta release notes, and > got lots of complaints, particularly because the line describing the > feature now had many more names on it. > > In my opinion, adding reviewer names to each feature item might result > in the removal of all names from features. > > A poll is nice for gauging interest, but many people who vote don't > understand the ramifications of what they are voting on. > That's why I voted for b :-) cheers andrew
On 06/25/2013 08:26 PM, Andres Freund wrote: > It's not about the reviewers being less. It's a comparison of > effort. The effort for a casual review simply isn't comparable with the > effort spent on developing a nontrivial patch. Remember: "Debugging is twice as hard as writing the code in the first place. ..." (Brian Kernighan) IMO, the kind of reviews we need are of almost "debug level" difficulty. (To the point where the reviewer becomes a co-author or even takes over and submits a completely revamped patch instead.) I agree that the casual review is several levels below that, so your point holds. I doubt we need more reviews of that kind, though. Thus, I'm in the AAB camp. The remaining question being: What's the criterion for becoming a co-author (and thus getting mentioned in the release notes)? If at all, we should honor quality work with a "prize". Maybe a price for the best reviewer per CF? Maybe even based on votes from the general public on who's been the best, so reviews gain attention that way... "Click here to vote for my review." ... Maybe a crazy idea. Regards Markus Wanner
Josh Berkus <josh@agliodbs.com> writes: > Well, one of the other "prizes" which occurred to me today would be a > pgCon lottery. That is, each review posted by a non-committer would go > in a hat, and in February we would draw one who would get a free > registration and airfare to pgCon. +1, I like that idea! Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
On Wed, Jun 26, 2013 at 10:25 AM, Andrew Dunstan <andrew@dunslane.net> wrote: > > On 06/26/2013 09:14 AM, Bruce Momjian wrote: >> >> On Wed, Jun 26, 2013 at 10:40:17AM +1000, Brendan Jurd wrote: >>> >>> On 26 June 2013 03:17, Josh Berkus <josh@agliodbs.com> wrote: >>>> >>>> How should reviewers get credited in the release notes? >>>> >>>> a) not at all >>>> b) in a single block titled "Reviewers for this version" at the bottom. >>>> c) on the patch they reviewed, for each patch >>> >>> A weak preference for (c), with (b) running a close second. As others >>> have suggested, a review that leads to significant commitable changes >>> to the patch should bump the credit to co-authorship. >> >> As a reminder, I tried a variant of C for 9.2 beta release notes, and >> got lots of complaints, particularly because the line describing the >> feature now had many more names on it. >> >> In my opinion, adding reviewer names to each feature item might result >> in the removal of all names from features. >> >> A poll is nice for gauging interest, but many people who vote don't >> understand the ramifications of what they are voting on. >> > > > That's why I voted for b :-) Yeah, with that in mind, I'd also switch to b. I wouldn't complain, but if it's been tried and failed... what can I say?
On Wed, 26 Jun 2013 09:14:07 -0400 Bruce Momjian <bruce@momjian.us> wrote: > On Wed, Jun 26, 2013 at 10:40:17AM +1000, Brendan Jurd wrote: > > On 26 June 2013 03:17, Josh Berkus <josh@agliodbs.com> wrote: > > > How should reviewers get credited in the release notes? > > > > > > a) not at all > > > b) in a single block titled "Reviewers for this version" at the > > > bottom. c) on the patch they reviewed, for each patch > > > > A weak preference for (c), with (b) running a close second. As > > others have suggested, a review that leads to significant > > commitable changes to the patch should bump the credit to > > co-authorship. > > As a reminder, I tried a variant of C for 9.2 beta release notes, and > got lots of complaints, particularly because the line describing the > feature now had many more names on it. I am just someone that is thinking that maybe can review things...I am not voting OK but I have a comment about your last email... If people thinks (and with people I am not talking about myself but regular committers and reviewers) think that option c is good, I think that we should change the tool (or the way) that release notes are done....I mean (and excuse my poor English) if people thing that it is the way to go, we should make tools good enough for what people want not change people thoughts cause tools are not good enough > > In my opinion, adding reviewer names to each feature item might result > in the removal of all names from features. Let's fix the way that release notes are done > > A poll is nice for gauging interest, but many people who vote don't > understand the ramifications of what they are voting on. > I agree, but cost-benefit is what we should see here not just cost....
On Wed, Jun 26, 2013 at 03:12:00PM -0300, Rodrigo Gonzalez wrote: > On Wed, 26 Jun 2013 09:14:07 -0400 > Bruce Momjian <bruce@momjian.us> wrote: > > > On Wed, Jun 26, 2013 at 10:40:17AM +1000, Brendan Jurd wrote: > > > On 26 June 2013 03:17, Josh Berkus <josh@agliodbs.com> wrote: > > > > How should reviewers get credited in the release notes? > > > > > > > > a) not at all > > > > b) in a single block titled "Reviewers for this version" at the > > > > bottom. c) on the patch they reviewed, for each patch > > > > > > A weak preference for (c), with (b) running a close second. As > > > others have suggested, a review that leads to significant > > > commitable changes to the patch should bump the credit to > > > co-authorship. > > > > As a reminder, I tried a variant of C for 9.2 beta release notes, and > > got lots of complaints, particularly because the line describing the > > feature now had many more names on it. > > I am just someone that is thinking that maybe can review things...I am > not voting OK but I have a comment about your last email... > If people thinks (and with people I am not talking about myself but > regular committers and reviewers) think that option c is good, I think > that we should change the tool (or the way) that release notes are > done....I mean (and excuse my poor English) if people thing that it is > the way to go, we should make tools good enough for what people want > not change people thoughts cause tools are not good enough Production of the release notes was not the problem; it was the text in the release notes. I don't see how we could modify the release note format. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On Wed, 26 Jun 2013 14:13:32 -0400 Bruce Momjian <bruce@momjian.us> wrote: > On Wed, Jun 26, 2013 at 03:12:00PM -0300, Rodrigo Gonzalez wrote: > > On Wed, 26 Jun 2013 09:14:07 -0400 > > Bruce Momjian <bruce@momjian.us> wrote: > > > > > On Wed, Jun 26, 2013 at 10:40:17AM +1000, Brendan Jurd wrote: > > > > On 26 June 2013 03:17, Josh Berkus <josh@agliodbs.com> wrote: > > > > > How should reviewers get credited in the release notes? > > > > > > > > > > a) not at all > > > > > b) in a single block titled "Reviewers for this version" at > > > > > the bottom. c) on the patch they reviewed, for each patch > > > > > > > > A weak preference for (c), with (b) running a close second. As > > > > others have suggested, a review that leads to significant > > > > commitable changes to the patch should bump the credit to > > > > co-authorship. > > > > > > As a reminder, I tried a variant of C for 9.2 beta release notes, > > > and got lots of complaints, particularly because the line > > > describing the feature now had many more names on it. > > > > I am just someone that is thinking that maybe can review things...I > > am not voting OK but I have a comment about your last email... > > If people thinks (and with people I am not talking about myself but > > regular committers and reviewers) think that option c is good, I > > think that we should change the tool (or the way) that release > > notes are done....I mean (and excuse my poor English) if people > > thing that it is the way to go, we should make tools good enough > > for what people want not change people thoughts cause tools are not > > good enough > > Production of the release notes was not the problem; it was the text > in the release notes. I don't see how we could modify the release > note format. > Well... Checking release notes for 9.2.4 you have Fix insecure parsing of server command-line switches (Mitsumasa Kondo, Kyotaro Horiguchi) What about (it people think that it is good) a second () with reviewed by <someone>....
On Wed, Jun 26, 2013 at 03:22:06PM -0300, Rodrigo Gonzalez wrote: > On Wed, 26 Jun 2013 14:13:32 -0400 > > Production of the release notes was not the problem; it was the text > > in the release notes. I don't see how we could modify the release > > note format. > > > > Well... > > Checking release notes for 9.2.4 > > you have Fix insecure parsing of server command-line switches > (Mitsumasa Kondo, Kyotaro Horiguchi) > > What about (it people think that it is good) a second () with reviewed > by <someone>.... That's what we had, and people didn't like it. If we overload that list of names, we might find we want to remove all the names. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
Bruce Momjian escribió: > On Wed, Jun 26, 2013 at 03:22:06PM -0300, Rodrigo Gonzalez wrote: > > Checking release notes for 9.2.4 > > > > you have Fix insecure parsing of server command-line switches > > (Mitsumasa Kondo, Kyotaro Horiguchi) > > > > What about (it people think that it is good) a second () with reviewed > > by <someone>.... > > That's what we had, and people didn't like it. If we overload that list > of names, we might find we want to remove all the names. Yeah, it becomes too long. (For security patches, in particular, it's probably not wise to list reviewers; there might well be reviewers whose input you never see because they happened in the closed security list). See the entry for foreign key locks: Prevent non-key-field row updates from locking foreign key rows (Álvaro Herrera, Noah Misch, Andres Freund, Alexander Shulgin,Marti Raudsepp) I am the author of most of the code, yet I chose to add Noah and Andres because they contributed a huge amount of time to reviewing the patch, and Alex and Marti because they submitted some code. They are all listed as coauthors, which seems a reasonable compromise to me. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On 06/26/2013 12:02 PM, Alvaro Herrera wrote: > See the entry for foreign key locks: > > Prevent non-key-field row updates from locking foreign key rows (Álvaro > Herrera, Noah Misch, Andres Freund, Alexander Shulgin, Marti Raudsepp) > > I am the author of most of the code, yet I chose to add Noah and Andres > because they contributed a huge amount of time to reviewing the patch, > and Alex and Marti because they submitted some code. They are all > listed as coauthors, which seems a reasonable compromise to me. What about the idea that reviewers who do code revision work, like in your FK patch, get listed after the original patch author with the patch, and reviewers do more lightweight reviews get listed at the bottom of the release notes? Seems fair to me. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
On Tue, Jun 25, 2013 at 10:17 AM, Josh Berkus <josh@agliodbs.com> wrote: > How should reviewers get credited in the release notes? Without getting into how we do this, I thought it might be helpful to share the reasons why I believe recognizing and expressing gratitude to reviewers is a helpful, useful and gratifying exercise for the Postgres community. I support crediting reviewers in a more formal way than we currently do for a few different reasons. First, I believe it's worth finding a way to say "Hey, you just did something great for Postgres", publicly, to a bunch of people who could have spent their valuable time and energy in some other way. Second, reviewers get better at their work by reviewing multiple times - so I'd like to encourage people to review more than once. Third, reviewers don't always need to be expert developers, or experts at Postgres to get started, but many people who do open source work have no idea this is true. Public recognition helps make it clear that we have people who give useful reviews and are relative novices. We also have several different kinds of reviews: * "does it compile" * style/typo/easily seen bug passes * in-depth discussion of design choices, use case, interface * complex testing cases * performance testing * pre-commit checks for more subtle bugs or committer preferences. All of those, except probably the very last, can be done by people who are familiar with Postgres or its configuration, but aren't necessarily Postgres or C experts. Fourth, we have very few accepted ways to recognize contributions to Postgres. Naming in Release Notes is one way this community has consistently supported as a *public* way to say "hey, you just did something great for Postgres". The complete list of ways I'm aware of are: 1. Recognizing major, minor and emeritus contributors 2. Making someone a committer 3. Being part of the -core group 4. Naming authors by name in commit messages (but without consistent metadata, making it difficult to count well) 5. Naming authors in release notes That's pretty much it. That's great for the people who have already secured positions through seniority, or because they're amazing C hackers. I don't know if I need to lay out for everyone the value of public recognition - if you want me to I can enumerate them. But the benefits of public recognition are huge -- both in a social and a financial sense. Currently, the only way I know of to be recognized for work on Postgres that is *not* seniority or code-related is #1. If you're a reviewer, there's almost no chance you'll be recognized in that way without some additional, very significant contribution to our community. (Please let me know if I'm mistaken about this -- I only know what I know!) Adding names to Release Notes (or some variant of Release Notes) seems like a minor concession for work done that we as a community value and want to encourage. We are so few in terms of patch contributors - somewhere between 300-400 people contribute code to PostgreSQL each year based on the names I try to pull out of commits. I haven't counted how many reviewers we have who do not also contribute code. Giving people appreciation for the review work they're doing for this community, for free, is a good thing for everyone. Naming more names helps describe the true scope of our community. Spreading gratitude is good for those who thank and those who receive thanks (like, proven scientifically!). And we increase the number of people who benefit directly from the work that they do here, by giving them something they can point their boss, their company and their colleagues to. So, when we're debating *how* recognition might be done, please don't lose sight of *why* this is important in the first place. -selena -- http://chesnok.com
On Thu, Jun 27, 2013 at 1:15 AM, Dimitri Fontaine <dimitri@2ndquadrant.fr> wrote: > Josh Berkus <josh@agliodbs.com> writes: >> Well, one of the other "prizes" which occurred to me today would be a >> pgCon lottery. That is, each review posted by a non-committer would go >> in a hat, and in February we would draw one who would get a free >> registration and airfare to pgCon. > > +1, I like that idea! +1 on that also, looks like an excellent idea. -- Michael
On 27/06/13 07:12, Josh Berkus wrote: > On 06/26/2013 12:02 PM, Alvaro Herrera wrote: >> See the entry for foreign key locks: >> >> Prevent non-key-field row updates from locking foreign key rows (Álvaro >> Herrera, Noah Misch, Andres Freund, Alexander Shulgin, Marti Raudsepp) >> >> I am the author of most of the code, yet I chose to add Noah and Andres >> because they contributed a huge amount of time to reviewing the patch, >> and Alex and Marti because they submitted some code. They are all >> listed as coauthors, which seems a reasonable compromise to me. > What about the idea that reviewers who do code revision work, like in > your FK patch, get listed after the original patch author with the > patch, and reviewers do more lightweight reviews get listed at the > bottom of the release notes? Seems fair to me. > I note reviewers are usually (?) mentioned in the commits to the git repo - so maybe it is enough to list them at the bottom of the release notes only? Regards Mark
On Tue, Jun 25, 2013 at 5:40 PM, Brendan Jurd <direvus@gmail.com> wrote:
On 26 June 2013 03:17, Josh Berkus <josh@agliodbs.com> wrote:A weak preference for (c), with (b) running a close second. As others
> How should reviewers get credited in the release notes?
>
> a) not at all
> b) in a single block titled "Reviewers for this version" at the bottom.
> c) on the patch they reviewed, for each patch
have suggested, a review that leads to significant commitable changes
to the patch should bump the credit to co-authorship.
I like the "single block at the bottom" myself, with the same provision to move a reviewer up to co-author.
> Should there be a criteria for a "creditable" review?(b), the review should at least look at usabililty, doc, and
>
> a) no, all reviews are worthwhile
> b) yes, they have to do more than "it compiles"
> c) yes, only code reviews should count
regression test criteria even if there is no in-depth code analysis.
+1 to this.
> Should reviewers for 9.4 get a "prize", such as a t-shirt, as a
> promotion to increase the number of non-submitter reviewers?
>
> a) yes
> b) no
> c) yes, but submitters and committers should get it too
I was going to go with b until I saw the suggestion for a PgCon ticket. I really like that idea.
gabrielle
gabrielle
On Tue, Jun 25, 2013 at 2:27 PM, Andrew Dunstan <andrew@dunslane.net> wrote: > I'd like to see prizes each release for "best contribution" and "best > reviewer" - I've thought for years something like this would be worth > trying. Committers and core members should not be eligible - this is about > encouraging new people. Encouraging new people is good, but recognizing sustained, long-term contributions is good, too. I think we should do more of that, too. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Thu, Jun 27, 2013 at 10:37 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Tue, Jun 25, 2013 at 2:27 PM, Andrew Dunstan <andrew@dunslane.net> wrote:Encouraging new people is good, but recognizing sustained, long-term
> I'd like to see prizes each release for "best contribution" and "best
> reviewer" - I've thought for years something like this would be worth
> trying. Committers and core members should not be eligible - this is about
> encouraging new people.
contributions is good, too. I think we should do more of that, too.
Conforming with David Fetter's pointer to the notion that sometimes attempts
to reward can backfire, I'm not sure that it will be super-helpful to create "special"
rewards.
to reward can backfire, I'm not sure that it will be super-helpful to create "special"
rewards.
On the other hand, to recognize reviewer contributions in places relevant to where
they take place seems pretty apropos, which could include:
a) Obviously we already capture this in the CommitFest web site (but it's
worth mentioning when trying to do a "census")
b) It would be a pretty good thing to mention reviewers within commit notes;
that provides some direct trace-back as to who it was that either validated
that the change was good, or that let a bad one slip through.
c) The release notes indicate authors of changes; to have a list of reviewers
they take place seems pretty apropos, which could include:
a) Obviously we already capture this in the CommitFest web site (but it's
worth mentioning when trying to do a "census")
b) It would be a pretty good thing to mention reviewers within commit notes;
that provides some direct trace-back as to who it was that either validated
that the change was good, or that let a bad one slip through.
c) The release notes indicate authors of changes; to have a list of reviewers
would be a fine thing.
If it requires inordinate effort to get the reviewers directly attached to each
and every change, perhaps it isn't worthwhile to go to extreme efforts to that
end.
end.
It could be pretty satisfactory to have a simple listing, in the release notes,
of the set of reviewers. That's a lot less bookkeeping than tracking this for
each and every change.
each and every change.
The statement of such a list is a public acknowledgement of those that help
assure that the quality of PostgreSQL code remains excellent. (And that may
represent a good way to sell this "kudo".)
represent a good way to sell this "kudo".)
This allows organizations that are sponsoring PostgreSQL development to
have an extra metric by which *they* can recognize that their staff that
do such work are being recognized as contributors. It seems to me that
this is way more useful than a free t-shirt or the like.
have an extra metric by which *they* can recognize that their staff that
do such work are being recognized as contributors. It seems to me that
this is way more useful than a free t-shirt or the like.
On Thu, Jun 27, 2013 at 11:50:07AM -0400, Christopher Browne wrote: > b) It would be a pretty good thing to mention reviewers within commit notes; > that provides some direct trace-back as to who it was that either validated > that the change was good, or that let a bad one slip through. > > c) The release notes indicate authors of changes; to have a list of reviewers > would be a fine thing. > > If it requires inordinate effort to get the reviewers directly attached to each > and every change, perhaps it isn't worthwhile to go to extreme efforts to that > end. > > It could be pretty satisfactory to have a simple listing, in the release notes, > of the set of reviewers. That's a lot less bookkeeping than tracking this for > each and every change. Adding the names to each release note item is not a problem; the problem is the volume of names that overwhelms the release note text. If we went that direction, I predict we would just remove _all_ names from the release notes. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
Bruce Momjian <bruce@momjian.us> writes:
> On Thu, Jun 27, 2013 at 11:50:07AM -0400, Christopher Browne wrote:
>> It could be pretty satisfactory to have a simple listing, in the
>> release notes, of the set of reviewers. That's a lot less
>> bookkeeping than tracking this for each and every change.
> Adding the names to each release note item is not a problem; the
> problem is the volume of names that overwhelms the release note text. If
> we went that direction, I predict we would just remove _all_ names from
> the release notes.
Yeah. Keep in mind that the overwhelming majority of the audience for
the release notes doesn't actually give a darn who implemented what.
regards, tom lane
On 06/27/2013 12:12 PM, Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: >> On Thu, Jun 27, 2013 at 11:50:07AM -0400, Christopher Browne wrote: >>> It could be pretty satisfactory to have a simple listing, in the >>> release notes, of the set of reviewers. That's a lot less >>> bookkeeping than tracking this for each and every change. >> Adding the names to each release note item is not a problem; the >> problem is the volume of names that overwhelms the release note text. If >> we went that direction, I predict we would just remove _all_ names from >> the release notes. > Yeah. Keep in mind that the overwhelming majority of the audience for > the release notes doesn't actually give a darn who implemented what. Maybe we should have a Kudos / Bragging rights wiki page, with a table something like this: Release Feature Name Principal Author(s) Contributing Author(s) Code Reviewer(s) Tester(s) Constructing it going backwards would be an interesting task :-) cheers andrew
On 6/25/13 2:44 PM, Josh Berkus wrote: > On the other hand, I will point out that we currently have a shortage of > reviewers, and we do NOT have a shortage of patch submitters. That's because reviewing is harder than initial development. The only people who think otherwise are developers who don't do enough review. -- Greg Smith 2ndQuadrant US greg@2ndQuadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.com
On 06/27/2013 08:56 AM, Bruce Momjian wrote:> Adding the names to each release note item is not a problem; the > problem is the volume of names that overwhelms the release note text. If > we went that direction, I predict we would just remove _all_ names from > the release notes. That's not a realistic objection. We'd be talking about one or two more names per patch, with a handful of exceptions. If you're going to make an objection, object to the amount of extra work it would be, which is signigficant with the current state of our technology. Realistically, it'll take the help of additional people. On 06/27/2013 09:19 AM, Tom Lane wrote: > Yeah. Keep in mind that the overwhelming majority of the audience for > the release notes doesn't actually give a darn who implemented what. There is some argument to be made about moving the whole "list of names" to some other resource, like a web page. In fact, if the mythical land where we have a new CF application, that other resource could even be generated by the CF app with some hand-tweaking (this would also require some tweaks to community profiles etc.). What I would be opposed to is continuing to list the original authors in the release notes and putting reviewers, testers, co-authors, etc. on a separate web page. If we're gonna move people, let's move *all* of them. Also, it needs to be on something more trustworthy than the wiki, like maybe putting it at postgresql.org/developers/9.3/ On Tue, Jun 25, 2013 at 2:27 PM, Andrew Dunstan <andrew@dunslane.net> wrote: > Encouraging new people is good, but recognizing sustained, long-term > contributions is good, too. I think we should do more of that, too. I wasn't thinking about doing it every year -- just for 9.3, in order to encourage more reviewers, and encourage reviewers to do more reviews. But that would only work if we're also giving reviewers credit; as several people have said, they care more about credit than they do about prizes. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
On Thu, Jun 27, 2013 at 10:10:23AM -0700, Josh Berkus wrote: > On 06/27/2013 08:56 AM, Bruce Momjian wrote:> Adding the names to each > release note item is not a problem; the > > problem is the volume of names that overwhelms the release note text. If > > we went that direction, I predict we would just remove _all_ names from > > the release notes. > > That's not a realistic objection. We'd be talking about one or two more > names per patch, with a handful of exceptions. It is realistic because I did this for 9.2 beta and people didn't like it; how much more realistic do you want? > If you're going to make an objection, object to the amount of extra work > it would be, which is significant with the current state of our > technology. Realistically, it'll take the help of additional people. No extra work required, it is all copy/paste for me. > On 06/27/2013 09:19 AM, Tom Lane wrote: > > Yeah. Keep in mind that the overwhelming majority of the audience for > > the release notes doesn't actually give a darn who implemented what. > > There is some argument to be made about moving the whole "list of names" > to some other resource, like a web page. In fact, if the mythical land > where we have a new CF application, that other resource could even be > generated by the CF app with some hand-tweaking (this would also require > some tweaks to community profiles etc.). Yes, I am fine with moving all names. However, I predict you will do more to demotivate patch submitters than you will to motivate patch reviewers. That is fine with me, as motivating developers/reviewers is not our top priority. > What I would be opposed to is continuing to list the original authors in > the release notes and putting reviewers, testers, co-authors, etc. on a > separate web page. If we're gonna move people, let's move *all* of > them. Also, it needs to be on something more trustworthy than the wiki, > like maybe putting it at postgresql.org/developers/9.3/ I think you will have trouble keeping those two lists synchronized. I think you will need to create a release note document that includes all names, then copy that to a website and remove the names just before the release is packaged. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
Bruce Momjian <bruce@momjian.us> writes:
> On Thu, Jun 27, 2013 at 10:10:23AM -0700, Josh Berkus wrote:
>> What I would be opposed to is continuing to list the original authors in
>> the release notes and putting reviewers, testers, co-authors, etc. on a
>> separate web page. If we're gonna move people, let's move *all* of
>> them. Also, it needs to be on something more trustworthy than the wiki,
>> like maybe putting it at postgresql.org/developers/9.3/
> I think you will have trouble keeping those two lists synchronized. I
> think you will need to create a release note document that includes all
> names, then copy that to a website and remove the names just before the
> release is packaged.
Unless Bruce is doing more work than I think he is, the attribution data
going into the release notes is just coming from whatever the committer
said in the commit log message. I believe that we've generally been
careful to credit the patch author, but I'm less confident that everyone
who merited a "review credit" always gets mentioned --- that would
require going through the entire review thread at commit time, and I for
one can't say that I do that.
If we're going to try harder to ensure that reviewers are credited,
it'd probably be better to take both the commit log and the release
notes out of that loop.
regards, tom lane
> If we're going to try harder to ensure that reviewers are credited, > it'd probably be better to take both the commit log and the release > notes out of that loop. I'd pull the reviewers out of the CF app. Even in its current implementation, that's the easiest route. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
On 06/27/2013 02:38 PM, Josh Berkus wrote: >> If we're going to try harder to ensure that reviewers are credited, >> it'd probably be better to take both the commit log and the release >> notes out of that loop. > I'd pull the reviewers out of the CF app. Even in its current > implementation, that's the easiest route. > I have not honestly found these to be terribly accurate. cheers andrew
On Thu, Jun 27, 2013 at 02:17:25PM -0400, Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > On Thu, Jun 27, 2013 at 10:10:23AM -0700, Josh Berkus wrote: > >> What I would be opposed to is continuing to list the original authors in > >> the release notes and putting reviewers, testers, co-authors, etc. on a > >> separate web page. If we're gonna move people, let's move *all* of > >> them. Also, it needs to be on something more trustworthy than the wiki, > >> like maybe putting it at postgresql.org/developers/9.3/ > > > I think you will have trouble keeping those two lists synchronized. I > > think you will need to create a release note document that includes all > > names, then copy that to a website and remove the names just before the > > release is packaged. > > Unless Bruce is doing more work than I think he is, the attribution data > going into the release notes is just coming from whatever the committer > said in the commit log message. I believe that we've generally been Yes, that's all I do. "Bruce is doing more work than I think he is" gave me a chuckle. ;-) > careful to credit the patch author, but I'm less confident that everyone > who merited a "review credit" always gets mentioned --- that would > require going through the entire review thread at commit time, and I for > one can't say that I do that. > > If we're going to try harder to ensure that reviewers are credited, > it'd probably be better to take both the commit log and the release > notes out of that loop. I assume you are suggesting we _not_ use the commit log and release notes for reviewer credit. Good point; we might be able to pull that from the commit-fest app, though you then need to match that with the release note text. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
Andrew Dunstan escribió: > > On 06/27/2013 02:38 PM, Josh Berkus wrote: > >>If we're going to try harder to ensure that reviewers are credited, > >>it'd probably be better to take both the commit log and the release > >>notes out of that loop. > >I'd pull the reviewers out of the CF app. Even in its current > >implementation, that's the easiest route. > > I have not honestly found these to be terribly accurate. Yeah, they're not. I do take care to review past threads when crediting reviewers in commit messages, and I don't even bother to look at the CF app. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
-----BEGIN PGP SIGNED MESSAGE----- Hash: RIPEMD160 Josh Berkus wrote: > I wasn't thinking about doing it every year -- just for 9.3, in order to > encourage more reviewers, and encourage reviewers to do more reviews. - -1. It's not cool to set it up and then stop it the next go round. You want more reviewers? Start by streamlining the process as much as possible. I pretended I was new to the project and tried to figure out how to review something. The homepage has no mention of reviewers, not even if you drill down on some subpages. A Google search does lead one to: http://wiki.postgresql.org/wiki/Reviewing_a_Patch It has some good "you can do it" wordage. However, there is no clear path on how to actually start reviewing. There is this paragraph with two links in it: "The current commitfest is here[1] and has plenty of room for you to help. You can sign up to become a Round Robin Reviewerhere[2]. Once you have, write a mail to the list introducing yourself." [1] Leads to the commitfest, with a nice summary, but no way for new people to know what to do. [2] This link is even worse (http://www.postgresql.org/list/pgsql-rrreviewers/) It's an archive list for pgsql-rrreviewers, with no way to subscribe and certainly no indication on it or the previous page that "sign up" means (one might guess) join the mailing list. Anyway, just food for thought as far as attracting new people. It should be much easier and more intuitive. As far as "rewarding" current reviewers, put the names in the release notes, after each item. Full stop. - -- Greg Sabino Mullane greg@turnstep.com End Point Corporation http://www.endpoint.com/ PGP Key: 0x14964AC8 201306271636 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -----BEGIN PGP SIGNATURE----- iEYEAREDAAYFAlHMoqIACgkQvJuQZxSWSsgCPACgovKYtxJV59Xro0MlxPDEHIy6 pmAAoOLOAlpO/dPlJbyHypdcY4ZxLCit =RwMh -----END PGP SIGNATURE-----
Folks, Well, I didn't get much in the way of "poll" responses for the straw poll. However, let me sum up: -- two hackers thought that reviewers didn't deserve any credit at all. -- of the majority of respondants, things were about evenly split between people who favored "big list at the end" and people who favored "reviewer next to feature". Notably, those who favored "reviewer next to feature" also thought that our standards for what constitutes a "review" should be more stringent. -- reviewers, in general, were unanimous that the only thing which mattered in terms of "rewarding" reviewers was credit in the release notes, and that other "rewards" were nice but inconsequential. -- a couple of compromise proposals were made: a) that reviewers who do actual code modification of the patch get credited on the feature, and those who just review it get credited at the bottom of the release notes, or b) that all "names" move to a web page on www.postgresql.org and come out of the release notes entirely. Speaking as a commitfest manager, I favor compromise proposal (a), personally. Does (a) seem somehow terrible to anyone? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
Josh Berkus wrote: > -- a couple of compromise proposals were made: > > a) that reviewers who do actual code modification of the patch get > credited on the feature, and those who just review it get credited at > the bottom of the release notes, or > > b) that all "names" move to a web page on www.postgresql.org and come > out of the release notes entirely. > > Speaking as a commitfest manager, I favor compromise proposal (a), > personally. Does (a) seem somehow terrible to anyone? I like (a) myself, though I'd amend it so that "extensive work on the patch" is required to qualify as co-author, which may or may not be code. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On 07/12/2013 01:28 PM, Alvaro Herrera wrote: > Josh Berkus wrote: > >> -- a couple of compromise proposals were made: >> >> a) that reviewers who do actual code modification of the patch get >> credited on the feature, and those who just review it get credited at >> the bottom of the release notes, or >> >> b) that all "names" move to a web page on www.postgresql.org and come >> out of the release notes entirely. >> >> Speaking as a commitfest manager, I favor compromise proposal (a), >> personally. Does (a) seem somehow terrible to anyone? > I like (a) myself, though I'd amend it so that "extensive work on the > patch" is required to qualify as co-author, which may or may not be > code. > I'd probably say "substantial" or "non-trivial", but otherwise +1 cheers andrew
On 07/12/2013 10:49 AM, Andrew Dunstan wrote: > > > On 07/12/2013 01:28 PM, Alvaro Herrera wrote: >> Josh Berkus wrote: >> >>> -- a couple of compromise proposals were made: >>> >>> a) that reviewers who do actual code modification of the patch get >>> credited on the feature, and those who just review it get credited at >>> the bottom of the release notes, or >>> > > I'd probably say "substantial" or "non-trivial", but otherwise +1 Right cause if a reviewer ends up writing (or cleaning up) all the docs, I would say they deserve very close to equal credit. As an example. JD -- Command Prompt, Inc. - http://www.commandprompt.com/ 509-416-6579 PostgreSQL Support, Training, Professional Services and Development High Availability, Oracle Conversion, Postgres-XC, @cmdpromptinc For my dreams of your image that blossoms a rose in the deeps of my heart. - W.B. Yeats
On Fri, Jul 12, 2013 at 12:18:15PM -0700, Joshua D. Drake wrote: > > On 07/12/2013 10:49 AM, Andrew Dunstan wrote: > > > > > >On 07/12/2013 01:28 PM, Alvaro Herrera wrote: > >>Josh Berkus wrote: > >> > >>>-- a couple of compromise proposals were made: > >>> > >>>a) that reviewers who do actual code modification of the patch get > >>>credited on the feature, and those who just review it get credited at > >>>the bottom of the release notes, or > >>> > > > > >I'd probably say "substantial" or "non-trivial", but otherwise +1 > > Right cause if a reviewer ends up writing (or cleaning up) all the > docs, I would say they deserve very close to equal credit. As an > example. I can do whatever we agree to in the release notes. The big question is whether committers can properly document these people. I do think the names are going to overwhelm the release note items and we will _again_ remove some or all names. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
Bruce Momjian wrote: > On Fri, Jul 12, 2013 at 12:18:15PM -0700, Joshua D. Drake wrote: > > Right cause if a reviewer ends up writing (or cleaning up) all the > > docs, I would say they deserve very close to equal credit. As an > > example. > > I can do whatever we agree to in the release notes. The big question > is whether committers can properly document these people. I don't see why not. Most of them, if not all, already do. > I do think the names are going to overwhelm the release note items and > we will _again_ remove some or all names. There's plenty of opinion to the contrary; but then it's just opinion. I think the idea of trying it at least once has merit. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On Fri, Aug 2, 2013 at 04:43:30PM -0400, Alvaro Herrera wrote: > Bruce Momjian wrote: > > On Fri, Jul 12, 2013 at 12:18:15PM -0700, Joshua D. Drake wrote: > > > > Right cause if a reviewer ends up writing (or cleaning up) all the > > > docs, I would say they deserve very close to equal credit. As an > > > example. > > > > I can do whatever we agree to in the release notes. The big question > > is whether committers can properly document these people. > > I don't see why not. Most of them, if not all, already do. Do they record which reviewers changed code and which just gave feedback? > > I do think the names are going to overwhelm the release note items and > > we will _again_ remove some or all names. > > There's plenty of opinion to the contrary; but then it's just opinion. > I think the idea of trying it at least once has merit. This is what the 9.2 release notes looked like before I remove the reviewers: http://momjian.us/expire/release-9-2.html Most items had 2-3 names, and it was widely rejected. Of course, these were all reviewers, not just those that changed the code. I did not have details of which reviewers changed code and which just gave feedback. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On 08/02/2013 01:56 PM, Bruce Momjian wrote: > On Fri, Aug 2, 2013 at 04:43:30PM -0400, Alvaro Herrera wrote: >> Bruce Momjian wrote: >>> On Fri, Jul 12, 2013 at 12:18:15PM -0700, Joshua D. Drake wrote: >> >>>> Right cause if a reviewer ends up writing (or cleaning up) all the >>>> docs, I would say they deserve very close to equal credit. As an >>>> example. >>> >>> I can do whatever we agree to in the release notes. The big question >>> is whether committers can properly document these people. >> >> I don't see why not. Most of them, if not all, already do. It is also my thinking that it is the job of the CommitFestManager to re-enforce this list by looking through the review list. If we do this on a per-CF basis, the workload won't become substantial; it's only if we wait until beta that it gets overwhelming. The CFM needs to supply the list of "reviewers at the end" anyway. > Most items had 2-3 names, and it was widely rejected. Of course, these > were all reviewers, not just those that changed the code. I did not > have details of which reviewers changed code and which just gave > feedback. I think "widely rejected" is an exaggeration; a few people objected stenuously. And the primary objection voiced was that people who did "it compiles!" shouldn't get equal credit with the original author of the patch. Which we're not proposing to do. BTW, all of this I'm talking about the 9.4 release notes, where we have the opportunity to start from the first CF. There's the question of what to do about the *9.3* release notes, which I'll address in a seperate email. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
Bruce, all: Per previous email, I wanted to make a specific proposal for what to do on the 9.3 release notes. This is because, without policy set, we have not been tracking which reviewers make "substantial changes" in 9.3, and listing some-but-not-all of them would cause a lot of unhappiness among the omitted reviewers. Therefore, I propose for 9.3 only that we just have the list at the end of the release notes, including all reviewers. That way we can make sure to include everyone equally. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
On Fri, Aug 2, 2013 at 02:10:17PM -0700, Josh Berkus wrote: > Bruce, all: > > Per previous email, I wanted to make a specific proposal for what to do > on the 9.3 release notes. This is because, without policy set, we have > not been tracking which reviewers make "substantial changes" in 9.3, and > listing some-but-not-all of them would cause a lot of unhappiness among > the omitted reviewers. > > Therefore, I propose for 9.3 only that we just have the list at the end > of the release notes, including all reviewers. That way we can make > sure to include everyone equally. Yes, there is no way to add people to the 9.3 release note items at this point --- my assumption is this is all 9.4 discussion, or maybe 9.5 as we have already committed quite a bit to 9.4. I don't think dumping reviewer names at the bottom of the 9.3 release notes is what the majority want, and it seems like an ugly short-term solution. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On Fri, Aug 2, 2013 at 02:07:53PM -0700, Josh Berkus wrote: > On 08/02/2013 01:56 PM, Bruce Momjian wrote: > > On Fri, Aug 2, 2013 at 04:43:30PM -0400, Alvaro Herrera wrote: > >> Bruce Momjian wrote: > >>> On Fri, Jul 12, 2013 at 12:18:15PM -0700, Joshua D. Drake wrote: > >> > >>>> Right cause if a reviewer ends up writing (or cleaning up) all the > >>>> docs, I would say they deserve very close to equal credit. As an > >>>> example. > >>> > >>> I can do whatever we agree to in the release notes. The big question > >>> is whether committers can properly document these people. > >> > >> I don't see why not. Most of them, if not all, already do. > > It is also my thinking that it is the job of the CommitFestManager to > re-enforce this list by looking through the review list. If we do this > on a per-CF basis, the workload won't become substantial; it's only if > we wait until beta that it gets overwhelming. Based on existing workflow, we need those reviewer names in the commit message. I don't see how the CommitFestManager can help with that. > The CFM needs to supply the list of "reviewers at the end" anyway. Why? > > Most items had 2-3 names, and it was widely rejected. Of course, these > > were all reviewers, not just those that changed the code. I did not > > have details of which reviewers changed code and which just gave > > feedback. > > I think "widely rejected" is an exaggeration; a few people objected > stenuously. And the primary objection voiced was that people who did > "it compiles!" shouldn't get equal credit with the original author of > the patch. Which we're not proposing to do. Well, I had to remove it pretty quickly, so that is my recolletion. > BTW, all of this I'm talking about the 9.4 release notes, where we have > the opportunity to start from the first CF. There's the question of what > to do about the *9.3* release notes, which I'll address in a seperate email. I am worried we are talking about 9.5 as we have already committed quite a bit to 9.4. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On 08/02/2013 02:24 PM, Bruce Momjian wrote: > Based on existing workflow, we need those reviewer names in the commit > message. I don't see how the CommitFestManager can help with that. We can change the workflow. It's ours, there's no government agency mandating it. Anyway, the list from the CFM would just be to make sure nobody got missed; it's a double-check on the commit messages. >> The CFM needs to supply the list of "reviewers at the end" anyway. > > Why? Who else would do it? >> BTW, all of this I'm talking about the 9.4 release notes, where we have >> the opportunity to start from the first CF. There's the question of what >> to do about the *9.3* release notes, which I'll address in a seperate email. > > I am worried we are talking about 9.5 as we have already committed quite > a bit to 9.4. You're making a big deal out of what's a minor clerical detail. Don't let minutia which any secretary could take care of get in the way of an important project goal, that is, rewarding reviewers so that lack of reviewers stops being a major project bottleneck. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
On 08/02/2013 02:23 PM, Bruce Momjian wrote: > I don't think dumping reviewer names at the bottom of the 9.3 release > notes is what the majority want, and it seems like an ugly short-term > solution. It's better than not crediting the reviewers *at all*, which is the only alternative I can think of. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
On Fri, Aug 2, 2013 at 02:36:42PM -0700, Josh Berkus wrote: > On 08/02/2013 02:24 PM, Bruce Momjian wrote: > > > Based on existing workflow, we need those reviewer names in the commit > > message. I don't see how the CommitFestManager can help with that. > > We can change the workflow. It's ours, there's no government agency > mandating it. > > Anyway, the list from the CFM would just be to make sure nobody got > missed; it's a double-check on the commit messages. > > >> The CFM needs to supply the list of "reviewers at the end" anyway. > > > > Why? > > Who else would do it? > > >> BTW, all of this I'm talking about the 9.4 release notes, where we have > >> the opportunity to start from the first CF. There's the question of what > >> to do about the *9.3* release notes, which I'll address in a seperate email. > > > > I am worried we are talking about 9.5 as we have already committed quite > > a bit to 9.4. > > You're making a big deal out of what's a minor clerical detail. Don't > let minutia which any secretary could take care of get in the way of an > important project goal, that is, rewarding reviewers so that lack of > reviewers stops being a major project bottleneck. You are approaching this like it is a done deal and everyone agrees to it. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On 08/02/2013 03:18 PM, Bruce Momjian wrote: >> You're making a big deal out of what's a minor clerical detail. Don't >> let minutia which any secretary could take care of get in the way of an >> important project goal, that is, rewarding reviewers so that lack of >> reviewers stops being a major project bottleneck. > > You are approaching this like it is a done deal and everyone agrees to > it. We already discussed it in the thread ad nauseum, and arrived at a compromise which everyone could live with. So from that perspective, it *is* a done deal, at least as far as 9.4 is concerned. At some point, we need to make a decision and move forward, instead of rehashing the same arguments forever. So if you're raising an objection to the compromise which many people already agreed to, then raise an objection and back it up. But don't sandbag. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
On Fri, Aug 2, 2013 at 03:55:27PM -0700, Josh Berkus wrote: > On 08/02/2013 03:18 PM, Bruce Momjian wrote: > >> You're making a big deal out of what's a minor clerical detail. Don't > >> let minutia which any secretary could take care of get in the way of an > >> important project goal, that is, rewarding reviewers so that lack of > >> reviewers stops being a major project bottleneck. > > > > You are approaching this like it is a done deal and everyone agrees to > > it. > > We already discussed it in the thread ad nauseum, and arrived at a > compromise which everyone could live with. So from that perspective, it > *is* a done deal, at least as far as 9.4 is concerned. At some point, > we need to make a decision and move forward, instead of rehashing the > same arguments forever. > > So if you're raising an objection to the compromise which many people > already agreed to, then raise an objection and back it up. But don't > sandbag. There are three issues here: 1. What will best motive reviewers? 2. What is a reasonable effort to accomplish #1? 3. What is acceptable for release note readers? You seem to be only focused on #1, and you don't want to address the other items --- that's fine --- I will still be around if people lose interest or the system becomes unworkable. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
Bruce, > There are three issues here: > > 1. What will best motive reviewers? > 2. What is a reasonable effort to accomplish #1? > 3. What is acceptable for release note readers? > > You seem to be only focused on #1, and you don't want to address the > other items --- that's fine --- I will still be around if people lose > interest or the system becomes unworkable. Both I and other people have already addressed #2 and #3. You're also having a huge failure of perspective here. Motivating reviewers allows our project to continue developing PostgreSQL. Items 2 and 3 are so insignificant in comparison to that as to not be worth discussing. In the novels The Hitchhiker's Guide To The Galaxy, there's a spaceship which has been waiting 1000 years to take off because it's waiting for a load of lemon-soaked paper napkins to be loaded. You are being Mr. Lemon-Soaked Paper Napkin. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
On 2013-08-07 10:04:08 -0700, Josh Berkus wrote: > Bruce, > > > There are three issues here: > > > > 1. What will best motive reviewers? > > 2. What is a reasonable effort to accomplish #1? > > 3. What is acceptable for release note readers? > > > > You seem to be only focused on #1, and you don't want to address the > > other items --- that's fine --- I will still be around if people lose > > interest or the system becomes unworkable. > > Both I and other people have already addressed #2 and #3. You're also > having a huge failure of perspective here. Motivating reviewers allows > our project to continue developing PostgreSQL. Items 2 and 3 are so > insignificant in comparison to that as to not be worth discussing. > > In the novels The Hitchhiker's Guide To The Galaxy, there's a spaceship > which has been waiting 1000 years to take off because it's waiting for a > load of lemon-soaked paper napkins to be loaded. You are being Mr. > Lemon-Soaked Paper Napkin. Holy crap Josh. I agree that we should give reviews space in the release notes, but could you please cut the ad-hominem bullshit? You're preventing your own success here. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On Wed, Aug 7, 2013 at 10:04:08AM -0700, Josh Berkus wrote: > Bruce, > > > There are three issues here: > > > > 1. What will best motive reviewers? > > 2. What is a reasonable effort to accomplish #1? > > 3. What is acceptable for release note readers? > > > > You seem to be only focused on #1, and you don't want to address the > > other items --- that's fine --- I will still be around if people lose > > interest or the system becomes unworkable. > > Both I and other people have already addressed #2 and #3. You're also > having a huge failure of perspective here. Motivating reviewers allows > our project to continue developing PostgreSQL. Items 2 and 3 are so > insignificant in comparison to that as to not be worth discussing. OK, my analysis was accurate then --- for you, #1 overshadows numbers 2 and 3. I don't share those priorities, and the work required is unbounded (no #2), so I will not perform additional work to help with #1. > In the novels The Hitchhiker's Guide To The Galaxy, there's a spaceship > which has been waiting 1000 years to take off because it's waiting for a > load of lemon-soaked paper napkins to be loaded. You are being Mr. > Lemon-Soaked Paper Napkin. Actually, for me, motiving reviewers seems like the Lemon-Soaked Paper Napkins, as it requires unbounded effort and its importance is not being balanced with other priorities. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On 08/07/2013 10:10 AM, Andres Freund wrote: > On 2013-08-07 10:04:08 -0700, Josh Berkus wrote: >> In the novels The Hitchhiker's Guide To The Galaxy, there's a spaceship >> which has been waiting 1000 years to take off because it's waiting for a >> load of lemon-soaked paper napkins to be loaded. You are being Mr. >> Lemon-Soaked Paper Napkin. > > Holy crap Josh. I agree that we should give reviews space in the release > notes, but could you please cut the ad-hominem bullshit? You're > preventing your own success here. Huh? I was comparing Bruce's *argument* to a scene in "Hitchhiker's Guide". How is that ad hominem? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
Bruce, > Actually, for me, motiving reviewers seems like the Lemon-Soaked Paper > Napkins, as it requires unbounded effort and its importance is not being > balanced with other priorities. Let me be absolutely clear here: You do not think that the work reviewers do is important at all, and you think that our project has more than enough reviewers? I want to be crystal-clear on your opinion. I will point out that you did exactly zero reviews in the last commitfest, which makes me wonder what your opinion of "we have enough reviewers" is based on. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
Josh,
* Josh Berkus (josh@agliodbs.com) wrote:
> > Actually, for me, motiving reviewers seems like the Lemon-Soaked Paper
> > Napkins, as it requires unbounded effort and its importance is not being
> > balanced with other priorities.
>
> Let me be absolutely clear here: You do not think that the work
> reviewers do is important at all, and you think that our project has
> more than enough reviewers? I want to be crystal-clear on your opinion.
Bruce certainly didn't say that and it's rather disingenuous to claim
that he did. What I read is that he simply pointed out that we have
multiple priorities and need to consider work on acquiring new
reviewers in balance with the rest.
Also mentioned is that it's unclear how one might bound the work of
getting new reviewers- you can't say "it'll take X hours to get enough
reviewers" or even "it'll take X hours to get 5 new reviewers".
Thanks,
Stephen
On Wed, Aug 7, 2013 at 12:07:32PM -0700, Josh Berkus wrote: > Bruce, > > > Actually, for me, motiving reviewers seems like the Lemon-Soaked Paper > > Napkins, as it requires unbounded effort and its importance is not being > > balanced with other priorities. > > Let me be absolutely clear here: You do not think that the work > reviewers do is important at all, and you think that our project has > more than enough reviewers? I want to be crystal-clear on your opinion. > > I will point out that you did exactly zero reviews in the last > commitfest, which makes me wonder what your opinion of "we have enough > reviewers" is based on. Only Lemon-Soaked Paper Napkins users think that everything is binary --- every effort has to be balanced against the work involved, which was #2 on my list. You are getting into some kind of loop where not wanting to expend unlimited effort on something means, to you, that the person doesn't think the goal is important. Effort has to be balanced. This is not the first time I have seen such loops. And why do you even care about my opinion? And I have never said "we have enough reviewers". Is this where you say I should be doing more? I thought we dealt with that already. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
Bruce, > You are getting into some kind of loop where not wanting to expend > unlimited effort on something means, to you, that the person doesn't > think the goal is important. Effort has to be balanced. This is not > the first time I have seen such loops. And why do you even care about > my opinion? Aha, OK. So you're talking about all the different things we might do to get more reviewers. I'm only talking about adding reviewers to the bottom of the release notes, which is certainly a bounded activity of *very* limited effort. Which is why I was confused and aghast at your talk of "unbounded work". To be completely clear: I am talking only about the compromise discussed on this thread, namely: a) listing reviewers who did "extensive work" as co-authors on the patch, and b) listing other reviewers at the bottom of the release notes. Per earlier discussions. You started this thread by claiming that adding the reviewers to 9.4 would be too hard, and I argued that it would not and in fact I'm already working on it. Nothing I've talked about in this thread has been about anything else. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
On 08/07/2013 10:35 AM, Bruce Momjian wrote: > Actually, for me, motiving reviewers seems like the Lemon-Soaked Paper > Napkins, as it requires unbounded effort and its importance is not being > balanced with other priorities. > Ignoring the non-productive part of this thread, I would like to mention that motivating reviewers is not necessarily complicated. We just have to ask ourselves what motivates a person: The feeling that their work is worthwhile, productive, will be appreciated and that they will receive recognition for the effort. Right now, we do not publicly outside of the dome that is -hackers provide those incentives. Give reviewers the just recognition they deserve and I believe we will see more reviewing effort. Sincerely, Joshua D. Drake -- Command Prompt, Inc. - http://www.commandprompt.com/ 509-416-6579 PostgreSQL Support, Training, Professional Services and Development High Availability, Oracle Conversion, Postgres-XC, @cmdpromptinc For my dreams of your image that blossoms a rose in the deeps of my heart. - W.B. Yeats
On Wed, Aug 7, 2013 at 12:39:01PM -0700, Josh Berkus wrote: > Bruce, > > > You are getting into some kind of loop where not wanting to expend > > unlimited effort on something means, to you, that the person doesn't > > think the goal is important. Effort has to be balanced. This is not > > the first time I have seen such loops. And why do you even care about > > my opinion? > > Aha, OK. So you're talking about all the different things we might do > to get more reviewers. I'm only talking about adding reviewers to the > bottom of the release notes, which is certainly a bounded activity of > *very* limited effort. Which is why I was confused and aghast at your > talk of "unbounded work". Well, reviewers on the bottom was just for 9.3 or 9.4, but the final goal was to get reviewers who modified patches credited with the release note items. I asked how that was to be accomplished, and suggested that the only practical way would be for every committer to check the patch chain to see who else had modified the patch. You suggested something about the commit-fest-manager doing it, and I couldn't see how that would help because it has to be in the commit message at the time the release notes are being written. You said our release note writing process was not written stone, and that we had to do whatever it takes to get those names on the items in the release notes. At that point I pointed out that there was no consideration of the effort necessary to accomplish this, and that's how we got here today. > To be completely clear: I am talking only about the compromise discussed > on this thread, namely: > > a) listing reviewers who did "extensive work" as co-authors on the > patch, and See above --- I need to know how that is going to get to the release note items _with_ reasonable effort. > b) listing other reviewers at the bottom of the release notes. Yes, that is somewhat easy in that we can get the names from the commit-fest app, but it doesn't include reviewers who replied via email but did not record their names on the commit-fest app. I can tell you from my release note writing experience that a partial job in this area is likely to get lots of negative feedback from people who are excluded. As an example, I got a pg_upgrade patch fix for 9.2, thought it was ugly, tried other methods, ended up doing the same thing the original patch author did, but didn't mention the patch author because I had forgotten I ended up with the same fix. When the minor release notes came out, the person complained on the hackers list, and Simon and Tom had to apologize as I was away: http://www.postgresql.org/message-id/CABRT9RAe3zxdins=BBysjqEPA8=NqnxWtA4A0XM01PbdVbmC_A@mail.gmail.com My point is this has to be done accurately. > Per earlier discussions. You started this thread by claiming that > adding the reviewers to 9.4 would be too hard, and I argued that it > would not and in fact I'm already working on it. Nothing I've talked > about in this thread has been about anything else. You have to distinguish between names at the end of the release notes, and names on release note items, and you have to tell us how this going to happen with reasonable effort. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
> Well, reviewers on the bottom was just for 9.3 or 9.4, but the final > goal was to get reviewers who modified patches credited with the release > note items. I asked how that was to be accomplished, and suggested that > the only practical way would be for every committer to check the patch > chain to see who else had modified the patch. Actually, it's not that hard. Someone who modified the patch is going to post it to -hackers, and we have that all in the archives. Michael and I are combing through the threads from CF1 now to get that list; you're talking a few hours effort at most. Of course, the actual patch author/committers need to verify that these people actually did a lot of *useful* work, but that isn't much effort from them. > You suggested something about the commit-fest-manager doing it, and I > couldn't see how that would help because it has to be in the commit > message at the time the release notes are being written. Why? You didn't provide *any* justification as to why the release notes could not include input *in addition to* commit messages. In fact, the release notes *do* incorporate additional input, every year. > You said our > release note writing process was not written stone, and that we had to > do whatever it takes to get those names on the items in the release > notes. At that point I pointed out that there was no consideration of > the effort necessary to accomplish this, and that's how we got here > today. The only additional effort I'm asking of you, Bruce, is to accept patches on the release notes. That really doesn't seem like an unreasonable request. > Yes, that is somewhat easy in that we can get the names from the > commit-fest app, but it doesn't include reviewers who replied via email > but did not record their names on the commit-fest app. I can tell you > from my release note writing experience that a partial job in this area > is likely to get lots of negative feedback from people who are > excluded. That's why it'll be a social process. Next week I'll be posting a list of patches and reviewers from CF1 to this list for other people to correct and expand. Just like the code itself, if everybody reviews the list of reviewers, it'll be as good as we can get it. > My point is this has to be done accurately. It will be just as accurate as the current process, which, as you've just pointed out, is not 100% accurate either. > You have to distinguish between names at the end of the release notes, > and names on release note items, and you have to tell us how this going > to happen with reasonable effort. You're making a mountain out of a molehill here. I've already spent more time arguing with you than it will take me to do the work. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
On Wed, Aug 7, 2013 at 01:48:06PM -0700, Josh Berkus wrote: > > > Well, reviewers on the bottom was just for 9.3 or 9.4, but the final > > goal was to get reviewers who modified patches credited with the release > > note items. I asked how that was to be accomplished, and suggested that > > the only practical way would be for every committer to check the patch > > chain to see who else had modified the patch. > > Actually, it's not that hard. Someone who modified the patch is going > to post it to -hackers, and we have that all in the archives. Michael > and I are combing through the threads from CF1 now to get that list; > you're talking a few hours effort at most. Michael who? 9.4 CF1? Where are you recording the names? In the commitfest app? > Of course, the actual patch author/committers need to verify that these > people actually did a lot of *useful* work, but that isn't much effort > from them. OK, so the process is independent of commit activity. You realize that if someone significantly modifies a patch we already have them in the commit message as an author and on the release note item, right? So you are really looking for reviews that modify the patch but not enough for a committer to include their name in the commit message as an author. > > You suggested something about the commit-fest-manager doing it, and I > > couldn't see how that would help because it has to be in the commit > > message at the time the release notes are being written. > > Why? You didn't provide *any* justification as to why the release notes > could not include input *in addition to* commit messages. In fact, the > release notes *do* incorporate additional input, every year. Yes, we can always add --- the problem is getting the content to add. > > You said our > > release note writing process was not written stone, and that we had to > > do whatever it takes to get those names on the items in the release > > notes. At that point I pointed out that there was no consideration of > > the effort necessary to accomplish this, and that's how we got here > > today. > > The only additional effort I'm asking of you, Bruce, is to accept > patches on the release notes. That really doesn't seem like an > unreasonable request. Anyone can commit patches to the release notes. I am unlikely to do it, as I lack confidence in the process, for reasons already outlined. > > Yes, that is somewhat easy in that we can get the names from the > > commit-fest app, but it doesn't include reviewers who replied via email > > but did not record their names on the commit-fest app. I can tell you > > from my release note writing experience that a partial job in this area > > is likely to get lots of negative feedback from people who are > > excluded. > > That's why it'll be a social process. Next week I'll be posting a list > of patches and reviewers from CF1 to this list for other people to > correct and expand. Just like the code itself, if everybody reviews the > list of reviewers, it'll be as good as we can get it. OK, at least that is a plan. Is that your plan for the future too? -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
> Michael who? Blackwell, asssistant CFM for this CF. > 9.4 CF1? Where are you recording the names? In the commitfest app? Right now in a googledoc. The CF app has no such capability now, although Magnus' new app might in the future. > OK, so the process is independent of commit activity. You realize that > if someone significantly modifies a patch we already have them in the > commit message as an author and on the release note item, right? So you > are really looking for reviews that modify the patch but not enough for > a committer to include their name in the commit message as an author. Oh, good point, I can look at the commit messages for where I don't need to bother. However, you pointed out that *during* CF1, the committers *didn't know* that they were supposed to include reviewers who did "major work" in the commit message. And they might miss them in the future. > Anyone can commit patches to the release notes. I am unlikely to do it, > as I lack confidence in the process, for reasons already outlined. Bruce, you are steadfastly resistant to change of any kind. >> That's why it'll be a social process. Next week I'll be posting a list >> of patches and reviewers from CF1 to this list for other people to >> correct and expand. Just like the code itself, if everybody reviews the >> list of reviewers, it'll be as good as we can get it. > > OK, at least that is a plan. Is that your plan for the future too? Sure. It's the open source way. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
On Wed, Aug 7, 2013 at 04:39:49PM -0700, Josh Berkus wrote: > > OK, so the process is independent of commit activity. You realize that > > if someone significantly modifies a patch we already have them in the > > commit message as an author and on the release note item, right? So you > > are really looking for reviews that modify the patch but not enough for > > a committer to include their name in the commit message as an author. > > Oh, good point, I can look at the commit messages for where I don't need > to bother. However, you pointed out that *during* CF1, the committers > *didn't know* that they were supposed to include reviewers who did > "major work" in the commit message. And they might miss them in the future. Well, the clear way to do this would be to ask the committers if they can reliably take on this job. You are right for CF1 they treated reviewer patch modifications just like anyone else, but of course, the larger question is whether you _should_ treat the reviewers different, because other people are reviewers just not recorded as CF reviewers. Another question is whether committers are going to recognize CF reviewers vs. ordinary patch modifiers. > > Anyone can commit patches to the release notes. I am unlikely to do it, > > as I lack confidence in the process, for reasons already outlined. > > Bruce, you are steadfastly resistant to change of any kind. Perhaps I am pessimistic, but I need to have confidence in the process, and at this point, I don't, and considering how long it took for me to get an explanation of the process, this seems prudent. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +