Re: [HACKERS] Commitfest problems
On 19/12/14 20:48, Andres Freund wrote: On 2014-12-18 10:02:25 -0800, Joshua D. Drake wrote: I think a lot of hackers forget exactly how tender their egos are. Now I say this knowing that a lot of them will go, Oh give me a break but as someone who employs hackers, deals with open source AND normal people :P every single day, I can tell you without a single inch of sarcasm that petting egos is one of the ways you get things done in the open source (and really any male dominated) community. To me that's a bit over the top stereotyping. +1 Having been mentioned one or two times myself - it was an unexpected wow - cool rather than a grumpy/fragile I must be noticed thing. I think some folk have forgotten the underlying principle of the open source community - it is about freely giving - time or code etc. The there must be something in it for me me me meme is - well - the *other* world view. However, doing crappy work and let's not be shy about it, there is NOTHING fun about reviewing someone else's code needs to have incentive. FWIW, I don't agree with this at all. Reviewing code can be quite interesting - with the one constraint that the problem the patch solves needs to be somewhat interesting. The latter is what I think gets many of the more experienced reviewers - lots of the patches just solve stuff they don't care about. Yeah, and also it helps if the patch addresses an area that you at least know *something* about - otherwise it is really hard to review in any useful way. Cheers Mark -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On Fri, Dec 19, 2014 at 6:28 AM, Mark Kirkwood mark.kirkw...@catalyst.net.nz wrote: On 19/12/14 20:48, Andres Freund wrote: On 2014-12-18 10:02:25 -0800, Joshua D. Drake wrote: I think a lot of hackers forget exactly how tender their egos are. Now I say this knowing that a lot of them will go, Oh give me a break but as someone who employs hackers, deals with open source AND normal people :P every single day, I can tell you without a single inch of sarcasm that petting egos is one of the ways you get things done in the open source (and really any male dominated) community. To me that's a bit over the top stereotyping. +1 Having been mentioned one or two times myself - it was an unexpected wow - cool rather than a grumpy/fragile I must be noticed thing. I think some folk have forgotten the underlying principle of the open source community - it is about freely giving - time or code etc. The there must be something in it for me me me meme is - well - the *other* world view. However, doing crappy work and let's not be shy about it, there is NOTHING fun about reviewing someone else's code needs to have incentive. FWIW, I don't agree with this at all. Reviewing code can be quite interesting - with the one constraint that the problem the patch solves needs to be somewhat interesting. The latter is what I think gets many of the more experienced reviewers - lots of the patches just solve stuff they don't care about. Yeah, and also it helps if the patch addresses an area that you at least know *something* about - otherwise it is really hard to review in any useful way. Cheers Mark -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers I'm trying to follow this thread as much as I could but I could've missed a part of it. Merit/credits aside what would really help Postgres right now is a more streamlined/modern (the only words I could come up with) development process. Using the mailing list for everything is alright, it works. But there is lot of tools that could be used along it: gerrit/gitlab/github/bitbucket/jira and other tools that do: pull requests, code review and bugs (or any combination of them). That'd reduce friction for new contributors and further development. These tools are being used everywhere and I find hard to believe that PG can't benefit from any of them. -- Arthur Silva
Re: [HACKERS] Commitfest problems
On Wed, Dec 17, 2014 at 02:00:18PM -0500, Stephen Frost wrote: Another thought I had was to suggest we consider *everyone* to be a contributor and implement a way to tie together the mailing list archives with the commit history and perhaps the commitfest app and make it searchable and indexed on some website. eg: contributors.postgresql.org/sfrost - Recent commits - Recent commit mentions - Recent emails to any list - Recent commitfest app activity - Recent wiki page updates ... Ideally with a way for individuals to upload a photo, provide a company link, etc, similar to what the existing Major Contributors have today. Obviously, this is not a small task to develop and there is some risk of abuse (which I expect the other folks on the infra team will point out and likely tar and feather me for suggesting this at all..) but it might be along the same lines as Bruce's PgLife.. The top of my Postgres blog page has some statistics for myself that might be useful for the community to maintain, and promote: http://momjian.us/main/blogs/pgblog/2014.html incoming, outgoing, unread, commits (details), events -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 12/19/2014 12:28 AM, Mark Kirkwood wrote: To me that's a bit over the top stereotyping. +1 Having been mentioned one or two times myself - it was an unexpected wow - cool rather than a grumpy/fragile I must be noticed thing. I think some folk have forgotten the underlying principle of the open source community - it is about freely giving - time or code etc. The there must be something in it for me me me meme is - well - the *other* world view. It was supposed to be over the top. That doesn't make it any less true. Sure there are plenty of us that don't have any of the ego petting issues. However,t there are more of us in those of us that think we don't, that really, really do. Sincerely, Joshua D. Drake -- Command Prompt, Inc. - http://www.commandprompt.com/ 503-667-4564 PostgreSQL Support, Training, Professional Services and Development High Availability, Oracle Conversion, @cmdpromptinc If we send our children to Caesar for their education, we should not be surprised when they come back as Romans. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 12/18/2014 05:36 PM, Stephen Frost wrote: I tend to agree that we want to avoid complicated rules. The corollary to that is the concern Andrew raised about my earlier off-the-cuff proposal- how do we avoid debasing the value of being recognized as a PG contributor? I find that less of a concern than recognizing all contributors. Let's not go crazy, but if we're going to err, it should be on the side of acknowledging *more* people, not less. is how to keep track of people that helps to which level. I make a point of crediting reviewers and code contributors in my commit messages, but can you tell which ones of the following guys should make it to these lists? I yanked this text from my commit 73c986adde5d73a5e2555da9b5c8facedb146dcd: Reviewed to varying degrees by Michael Paquier, Andres Freund, Robert Haas, Amit Kapila, Fujii Masao, Jaime Casanova, Simon Riggs, Steven Singer, Peter Eisentraut I do agree that we need to give credit in some form, though. I'm just saying can we please not put the responsibility on committers. Ugh, yeah, I certainly wouldn't want to have to work out some complex set of rules to be applied before each commit to define who can be considered a reviewer. That said, most of the above list are committers and those who aren't should be recognized in some fashion, so I'm not sure that this is really a good example. This really doesn't sound that difficult. If the committers include all actual reviewers in the commit messages, then it will be fairly easy for someone else (me) to go through and pick out the relative handful of people who aren't already on the contributors list and check the level of their contributions. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
Josh Berkus j...@agliodbs.com writes: On 12/18/2014 05:36 PM, Stephen Frost wrote: I do agree that we need to give credit in some form, though. I'm just saying can we please not put the responsibility on committers. Ugh, yeah, I certainly wouldn't want to have to work out some complex set of rules to be applied before each commit to define who can be considered a reviewer. That said, most of the above list are committers and those who aren't should be recognized in some fashion, so I'm not sure that this is really a good example. This really doesn't sound that difficult. If the committers include all actual reviewers in the commit messages, then it will be fairly easy for someone else (me) to go through and pick out the relative handful of people who aren't already on the contributors list and check the level of their contributions. I'm with Alvaro: I don't mind copying the commitfest app's credits list into commit messages, but please don't ask me to review who should get credit or not. If I have to do it, it's going to be done hastily at the tail end of the commit process, and probably not very well; and once the commit is made it's not very practical to fix any mistakes. Could we establish an expectation that whoever sets a CF entry to ready for committer is responsible for reviewing the authors/reviewers lists and making sure that those fairly represent who should get credit? That would divide the labor a bit, and there would also be time enough for corrections if anyone feels slighted. The idea's not perfect since major contributions could still happen after that point; but I think the major risk is with the committer not remembering people who contributed early in the patch's life cycle, and we could certainly hope that such people get listed in the CF app entry. Alternatively we could abandon the practice of using the commit log for this purpose, which could simplify making after-the-fact corrections. But then we'd have to set up some other recording infrastructure and work flow for getting the info into the release notes. That sounds like a lot of work compared to the probable value. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 12/19/14, 6:16 PM, Tom Lane wrote: Could we establish an expectation that whoever sets a CF entry to ready for committer is responsible for reviewing the authors/reviewers lists and making sure that those fairly represent who should get credit? That would divide the labor a bit, and there would also be time enough for corrections if anyone feels slighted. The idea's not perfect since major contributions could still happen after that point; but I think the major risk is with the committer not remembering people who contributed early in the patch's life cycle, and we could certainly hope that such people get listed in the CF app entry. Perhaps go even one step further and let a reviewer draft the actual commit message? That would further reduce committer workload, assuming the committer agrees with the draft commit message. Alternatively we could abandon the practice of using the commit log for this purpose, which could simplify making after-the-fact corrections. But then we'd have to set up some other recording infrastructure and work flow for getting the info into the release notes. That sounds like a lot of work compared to the probable value. git does allow you to revise a commit message; it just makes downstream pulls uglier if the commit was already pushed (see https://help.github.com/articles/changing-a-commit-message/). It might be possible to minimize or even eliminate that pain via git hooks. -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 2014-12-19 22:17:54 -0600, Jim Nasby wrote: git does allow you to revise a commit message; it just makes downstream pulls uglier if the commit was already pushed (see https://help.github.com/articles/changing-a-commit-message/). It might be possible to minimize or even eliminate that pain via git hooks. That's completely not acceptable for anything used by others. What can sanely changed after the fact is 'git notes'. With that notes to commits can be added, edited and changed after the original commit. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 20/12/14 11:22, Joshua D. Drake wrote: On 12/19/2014 12:28 AM, Mark Kirkwood wrote: To me that's a bit over the top stereotyping. +1 Having been mentioned one or two times myself - it was an unexpected wow - cool rather than a grumpy/fragile I must be noticed thing. I think some folk have forgotten the underlying principle of the open source community - it is about freely giving - time or code etc. The there must be something in it for me me me meme is - well - the *other* world view. It was supposed to be over the top. That doesn't make it any less true. Sure there are plenty of us that don't have any of the ego petting issues. However,t there are more of us in those of us that think we don't, that really, really do. Heh - that fact that even you are stating it is over the top clearly makes it less *generally* true. Sure, there are some/few(?) folk who are seeing open source contributions as purely a CV enhancer, and perhaps a few in denial about their egos, but I don't see that as the common trend in this community (which is great). regards Mark -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 16.12.2014 08:33, David Rowley wrote: On 16 December 2014 at 18:18, Josh Berkus j...@agliodbs.com mailto:j...@agliodbs.com wrote: Man. You're equating stuff that's not the same. You didn't get your way (and I'm tentatively on your side onthat one) and take that to imply that we don't want more reviewers. During that thread a couple people said that novice reviewers added no value to the review process, and nobody argued with them then. I've also been told this to my face at pgCon, and when I've tried organizing patch review events. I got the message, which is why I stopped trying to get new reviewers. And frankly: if we're opposed to giving credit to patch reviewers, we're opposed to having them. I'd just like to add something which might be flying below the radar of more senior people. There are people out there (ike me) working on PostgreSQL more for the challenge and perhaps the love of the product, who make absolutely zero money out of it. For these people getting credit where it's due is very important. I'm pretty happy with this at the moment and I can't imagine any situation where not crediting reviewers would be beneficial to anyone. I want to focus this point a little more. I'm educate young people to become developers and i always try to encourage them to participate in open source projects for many reasons. In my experience people are very different and one of the key arguments to make some becoming part of a community is the credit the can earn. Finding their own name in the release-note of a known project is very good for the ego and one of their main motivations. This is especially import for *young* people. Also its easy to argue that credits makes it easier to get a good job later. If you can write have a look at the release notes of $x, i make the feature you're using in your references of a application for employment this is a big plus for you. People are very different, but there are some groups of motivations to address. For me its very unimportant if you name me or not. 10 years ago i would not have participate in anything without somebody stretching out this was my genius work :( Apart of this social addressing problem i believe there are some other problems in the process of reviewing patches in the context of novice reviewers. I'm trying to write it in a separate note. Greetings, Torsten -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 17.12.2014 20:00, Stephen Frost wrote: * Jaime Casanova (ja...@2ndquadrant.com) wrote: On Tue, Dec 16, 2014 at 11:32 AM, Robert Haas robertmh...@gmail.com wrote: It has been proposed that we do a general list of people at the bottom of the release notes who helped review during that cycle. That would be less intrusive and possibly a good idea, but would we credit the people who did a TON of reviewing? Everyone who reviewed even one patch? Somewhere in between? Would committers be excluded because we just expect them to help or included because credit is important to established community members too? To what extent would this be duplicative of http://www.postgresql.org/community/contributors/ ? Not much really, I tried to get my name on that list a couple of years ago, when i reviewed more than i do now, and never got it. And while my name is in a couple commit messages, that is a lot harder to show to people... Having your name in a list of other names at the bottom of the release notes page, without any indication of what you helped with, would work better? Perhaps it would but I tend to doubt it. Out of my personal experience in Germany: yes, it helps. It is not very logical, but many people need a simple way (Website against git log) to see something. (I've rarely seen that something like that is considered not trustable even if there are strong indications that its faked.) But i think it is a good point that the release notes should not become to big. you know, it's kind of frustrating when some not-yet customers ask for certificated engineers, and there isn't any official (as in from community) certificate so you need to prove you're a contributor so let's see this random commit messages... Another thought I had was to suggest we consider *everyone* to be a contributor and implement a way to tie together the mailing list archives with the commit history and perhaps the commitfest app and make it searchable and indexed on some website. eg: contributors.postgresql.org/sfrost - Recent commits - Recent commit mentions - Recent emails to any list - Recent commitfest app activity - Recent wiki page updates ... Ideally with a way for individuals to upload a photo, provide a company link, etc, similar to what the existing Major Contributors have today. Obviously, this is not a small task to develop and there is some risk of abuse (which I expect the other folks on the infra team will point out and likely tar and feather me for suggesting this at all..) but it might be along the same lines as Bruce's PgLife.. That's an interesting idea. I'm not convinced that this is the best solution, but i would help, if community thinks this should be implemented. Greetings, Torsten -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On Wed, Dec 17, 2014 at 5:00 PM, Stephen Frost sfr...@snowman.net wrote: Another thought I had was to suggest we consider *everyone* to be a contributor and implement a way to tie together the mailing list archives with the commit history and perhaps the commitfest app and make it searchable and indexed on some website. eg: contributors.postgresql.org/sfrost - Recent commits - Recent commit mentions - Recent emails to any list - Recent commitfest app activity - Recent wiki page updates ... Ideally with a way for individuals to upload a photo, provide a company link, etc, similar to what the existing Major Contributors have today. Obviously, this is not a small task to develop and there is some risk of abuse (which I expect the other folks on the infra team will point out and likely tar and feather me for suggesting this at all..) but it might be along the same lines as Bruce's PgLife.. +1 Here in Brazil we need a better way to proof our contributions an involvement with the PostgreSQL Community. Regards, -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL Timbira: http://www.timbira.com.br Blog: http://fabriziomello.github.io Linkedin: http://br.linkedin.com/in/fabriziomello Twitter: http://twitter.com/fabriziomello Github: http://github.com/fabriziomello
Re: [HACKERS] Commitfest problems
On Thu, Dec 18, 2014 at 8:44 PM, Fabrízio de Royes Mello fabriziome...@gmail.com wrote: On Wed, Dec 17, 2014 at 5:00 PM, Stephen Frost sfr...@snowman.net wrote: Another thought I had was to suggest we consider *everyone* to be a contributor and implement a way to tie together the mailing list archives with the commit history and perhaps the commitfest app and make it searchable and indexed on some website. eg: contributors.postgresql.org/sfrost - Recent commits - Recent commit mentions - Recent emails to any list - Recent commitfest app activity - Recent wiki page updates ... Ideally with a way for individuals to upload a photo, provide a company link, etc, similar to what the existing Major Contributors have today. Obviously, this is not a small task to develop and there is some risk of abuse (which I expect the other folks on the infra team will point out and likely tar and feather me for suggesting this at all..) but it might be along the same lines as Bruce's PgLife.. +1 It does feel good to be acknowledged for our work especially when there is a policy to acknowledge this in our community. Regards, Atri
Re: [HACKERS] Commitfest problems
On 12/18/2014 04:53 AM, Torsten Zuehlsdorff wrote: Having your name in a list of other names at the bottom of the release notes page, without any indication of what you helped with, would work better? Perhaps it would but I tend to doubt it. Out of my personal experience in Germany: yes, it helps. It is not very logical, but many people need a simple way (Website against git log) to see something. (I've rarely seen that something like that is considered not trustable even if there are strong indications that its faked.) But i think it is a good point that the release notes should not become to big. I think a lot of hackers forget exactly how tender their egos are. Now I say this knowing that a lot of them will go, Oh give me a break but as someone who employs hackers, deals with open source AND normal people :P every single day, I can tell you without a single inch of sarcasm that petting egos is one of the ways you get things done in the open source (and really any male dominated) community. The problem is, most of our long term contributers don't need to be petted quite so often because they have a long list of: I don't need my ego stroked, do you see the length or value of the contributions I provide? And simply, there are some that just don't care. However, doing crappy work and let's not be shy about it, there is NOTHING fun about reviewing someone else's code needs to have incentive. Just like when we were kids, we were much more likely to rake the leaves with at least a half smile if we got that extra 10 bucks or if we were able to go to that party on Friday. Finding a way to provide incentive and credit (and they may be the same) will increase the value of the non-self value work of reviewing patches. In the the Pg world, the most obvious way is to have attribution in a public space. Perhaps an email that goes out to -announce (and planet) for each release that is a thank you to contributors? That way we don't touch the release notes at all. Sincerely, Joshua D. Drake -- Command Prompt, Inc. - http://www.commandprompt.com/ 503-667-4564 PostgreSQL Support, Training, Professional Services and Development High Availability, Oracle Conversion, @cmdpromptinc If we send our children to Caesar for their education, we should not be surprised when they come back as Romans. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 12/18/2014 07:31 AM, Andrew Dunstan wrote: +1 It does feel good to be acknowledged for our work especially when there is a policy to acknowledge this in our community. I like this idea but who is going to code our new social network? Frankly, this coin is going to become so debased as to be worthless. From a specific desire to acknowledge reviewers for their work we are now getting to a list that you can get on by posting to any mailing list. Sure you can but that doesn't mean there is value in the fact that they are listed. Just like Facebook, it is all about how many friends you have or linked-in who has the endorse feature. Please, can we stick to what was the original point. This tendency we have to enlarge proposals way beyond their origin is why getting things done is often so difficult. Good point. JD -- Command Prompt, Inc. - http://www.commandprompt.com/ 503-667-4564 PostgreSQL Support, Training, Professional Services and Development High Availability, Oracle Conversion, @cmdpromptinc If we send our children to Caesar for their education, we should not be surprised when they come back as Romans. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
All, It's sounding like folks would prefer keeing the master contributors list up to date, to adding a bunch of names to the release notes. So, then, I have a proposal for criteria for getting on the contributors list via patch review: - substantial, deep review of at least one patch (including detailed code review and possible corrections) - functionality reviews of at least 3 patches, including full write-ups (not just it compiled, seems to work). Kibitz as you may, but please don't try to make these criteria more *complicated*, because there's no way we'll ever keep track. Note that updating the contributor list in the past has been slow due to lack of technology and process; if it's our way forward, then I'll apply myself to that problem. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
Josh Berkus wrote: So, then, I have a proposal for criteria for getting on the contributors list via patch review: - substantial, deep review of at least one patch (including detailed code review and possible corrections) - functionality reviews of at least 3 patches, including full write-ups (not just it compiled, seems to work). Kibitz as you may, but please don't try to make these criteria more *complicated*, because there's no way we'll ever keep track. The problem with complicated rules (which these, I think, already are) is how to keep track of people that helps to which level. I make a point of crediting reviewers and code contributors in my commit messages, but can you tell which ones of the following guys should make it to these lists? I yanked this text from my commit 73c986adde5d73a5e2555da9b5c8facedb146dcd: Reviewed to varying degrees by Michael Paquier, Andres Freund, Robert Haas, Amit Kapila, Fujii Masao, Jaime Casanova, Simon Riggs, Steven Singer, Peter Eisentraut I do agree that we need to give credit in some form, though. I'm just saying can we please not put the responsibility on committers. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 12/18/2014 10:37 AM, Alvaro Herrera wrote: The problem with complicated rules (which these, I think, already are) is how to keep track of people that helps to which level. I make a point of crediting reviewers and code contributors in my commit messages, but can you tell which ones of the following guys should make it to these lists? I yanked this text from my commit 73c986adde5d73a5e2555da9b5c8facedb146dcd: Reviewed to varying degrees by Michael Paquier, Andres Freund, Robert Haas, Amit Kapila, Fujii Masao, Jaime Casanova, Simon Riggs, Steven Singer, Peter Eisentraut I do agree that we need to give credit in some form, though. I'm just saying can we please not put the responsibility on committers. Truthfully, I think this is something that should be solved through crowdsourcing. Why is a relatively small group of people (-hackers) trying to solve a problem that each individual can solve on their own given the tools? Allow people to submit for approval their own contributor listing, allow people to edit for approval their own contributor listing. Just like news/events. If people want to be listed they can be, it just has to be approved. Then have a very basic (not unlike what JB just posted) rules for the moderators to review against. JD -- Command Prompt, Inc. - http://www.commandprompt.com/ 503-667-4564 PostgreSQL Support, Training, Professional Services and Development High Availability, Oracle Conversion, @cmdpromptinc If we send our children to Caesar for their education, we should not be surprised when they come back as Romans. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 19/12/14 07:02, Joshua D. Drake wrote: On 12/18/2014 04:53 AM, Torsten Zuehlsdorff wrote: Having your name in a list of other names at the bottom of the release notes page, without any indication of what you helped with, would work better? Perhaps it would but I tend to doubt it. Out of my personal experience in Germany: yes, it helps. It is not very logical, but many people need a simple way (Website against git log) to see something. (I've rarely seen that something like that is considered not trustable even if there are strong indications that its faked.) But i think it is a good point that the release notes should not become to big. I think a lot of hackers forget exactly how tender their egos are. Now I say this knowing that a lot of them will go, Oh give me a break but as someone who employs hackers, deals with open source AND normal people :P every single day, I can tell you without a single inch of sarcasm that petting egos is one of the ways you get things done in the open source (and really any male dominated) community. The problem is, most of our long term contributers don't need to be petted quite so often because they have a long list of: I don't need my ego stroked, do you see the length or value of the contributions I provide? And simply, there are some that just don't care. However, doing crappy work and let's not be shy about it, there is NOTHING fun about reviewing someone else's code needs to have incentive. Just like when we were kids, we were much more likely to rake the leaves with at least a half smile if we got that extra 10 bucks or if we were able to go to that party on Friday. Finding a way to provide incentive and credit (and they may be the same) will increase the value of the non-self value work of reviewing patches. In the the Pg world, the most obvious way is to have attribution in a public space. Perhaps an email that goes out to -announce (and planet) for each release that is a thank you to contributors? That way we don't touch the release notes at all. Sincerely, Joshua D. Drake Hey Joshua, what does a 'Normal person look like??? :-) I did help review some code for a pg contributor once, but it was very minor. If I was 17, I would probably get a kick out of seeing my name mentioned - but now I would be embarrassed, because what I did was was quite insignificant in the scale of things. I think a separate list of contributors would be good. How about some 'Browne points' mechanism which would give a rough measure of the value of the contribution. My contribution would probably rate '1', whereas Tom's would be at least '100,000,000' - more realistically: my contribution would not rate at all, but Tom's would still be the largest by far! Perhaps a log scale so Tom would not show us up so much, and the contributions of new people would look more significant? Probably any such scheme would be too difficult to administer in practice, or taken far too seriously, though it might be fun. Cheers, Gavin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 12/18/2014 11:03 AM, Gavin Flower wrote: Hey Joshua, what does a 'Normal person look like??? :-) Hahhhahahah, you have to get out of your basement to see them. Usually, they are at the latest and newest coffee hub, talking about hating hipsters while wearing skinny jeans and a new but used looking plaid shirt. I think a separate list of contributors would be good. How about some 'Browne points' mechanism which would give a rough measure of the value of the contribution. My contribution would probably rate '1', whereas Tom's would be at least '100,000,000' - more realistically: my contribution would not rate at all, but Tom's would still be the largest by far! Perhaps a log scale so Tom would not show us up so much, and the contributions of new people would look more significant? Probably any such scheme would be too difficult to administer in practice, or taken far too seriously, though it might be fun. I don't think it would be difficult to come up with a basic system that gives weight to length of contribution as well as quantity and quality. But I don't think your specific applies. Tom is core, that in itself is the platinum badge of honor from an outsiders perspective. Sincerely, Joshua D. Drake -- Command Prompt, Inc. - http://www.commandprompt.com/ 503-667-4564 PostgreSQL Support, Training, Professional Services and Development High Availability, Oracle Conversion, @cmdpromptinc If we send our children to Caesar for their education, we should not be surprised when they come back as Romans. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 12/18/14, 12:08 PM, Joshua D. Drake wrote: It does feel good to be acknowledged for our work especially when there is a policy to acknowledge this in our community. I like this idea but who is going to code our new social network? +1. I do like the idea; but I don't like it enough to do it myself. :) -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
* Andrew Dunstan (and...@dunslane.net) wrote: On Wed, Dec 17, 2014 at 5:00 PM, Stephen Frost sfr...@snowman.net mailto:sfr...@snowman.net wrote: contributors.postgresql.org/sfrost http://contributors.postgresql.org/sfrost - Recent commits - Recent commit mentions - Recent emails to any list - Recent commitfest app activity - Recent wiki page updates Frankly, this coin is going to become so debased as to be worthless. We could manage that. I certainly feel like a lot *more* folks should be acknowledged than we do today, if the feeling is that the commit message acknowledgments are insufficient. Would it be possible to articulate the criteria which would be sufficient for inclusion, to avoid having it be debased? From a specific desire to acknowledge reviewers for their work we are now getting to a list that you can get on by posting to any mailing list. I was thinking it'd take a bit more than that- perhaps regular emails and you have to ask for it? Or you have to have been mentioned in the commit history somewhere? Perhaps not obvious but implied was a requirement to have a community account. Please, can we stick to what was the original point. This tendency we have to enlarge proposals way beyond their origin is why getting things done is often so difficult. I agree with your concern that this could turn into a large effort which would derail the main discussion, but the above question is a good one- how do we avoid debaseing the value of inclusion in anything like this (or in the release notes..) while still providing the recognition and credit that the individuals who are helping to make PG the great system it is deserve? Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Commitfest problems
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: Josh Berkus wrote: So, then, I have a proposal for criteria for getting on the contributors list via patch review: - substantial, deep review of at least one patch (including detailed code review and possible corrections) - functionality reviews of at least 3 patches, including full write-ups (not just it compiled, seems to work). Kibitz as you may, but please don't try to make these criteria more *complicated*, because there's no way we'll ever keep track. The problem with complicated rules (which these, I think, already are) I tend to agree that we want to avoid complicated rules. The corollary to that is the concern Andrew raised about my earlier off-the-cuff proposal- how do we avoid debasing the value of being recognized as a PG contributor? is how to keep track of people that helps to which level. I make a point of crediting reviewers and code contributors in my commit messages, but can you tell which ones of the following guys should make it to these lists? I yanked this text from my commit 73c986adde5d73a5e2555da9b5c8facedb146dcd: Reviewed to varying degrees by Michael Paquier, Andres Freund, Robert Haas, Amit Kapila, Fujii Masao, Jaime Casanova, Simon Riggs, Steven Singer, Peter Eisentraut I do agree that we need to give credit in some form, though. I'm just saying can we please not put the responsibility on committers. Ugh, yeah, I certainly wouldn't want to have to work out some complex set of rules to be applied before each commit to define who can be considered a reviewer. That said, most of the above list are committers and those who aren't should be recognized in some fashion, so I'm not sure that this is really a good example. I don't have a good example of someone mentioned as a reviewer in the git message but who doesn't deserve recognition though and I'm actually not sure that we even have such an example in our git history.. If so, well, I'd rather err on the side of being more inclusive than less inclusive anyway. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Commitfest problems
* Jim Nasby (jim.na...@bluetreble.com) wrote: On 12/18/14, 12:08 PM, Joshua D. Drake wrote: It does feel good to be acknowledged for our work especially when there is a policy to acknowledge this in our community. I like this idea but who is going to code our new social network? +1. I do like the idea; but I don't like it enough to do it myself. :) I figured Magnus would be all over it.. ;D /me ducks runs Stephen signature.asc Description: Digital signature
Re: [HACKERS] Commitfest problems
On 2014-12-18 10:02:25 -0800, Joshua D. Drake wrote: I think a lot of hackers forget exactly how tender their egos are. Now I say this knowing that a lot of them will go, Oh give me a break but as someone who employs hackers, deals with open source AND normal people :P every single day, I can tell you without a single inch of sarcasm that petting egos is one of the ways you get things done in the open source (and really any male dominated) community. To me that's a bit over the top stereotyping. However, doing crappy work and let's not be shy about it, there is NOTHING fun about reviewing someone else's code needs to have incentive. FWIW, I don't agree with this at all. Reviewing code can be quite interesting - with the one constraint that the problem the patch solves needs to be somewhat interesting. The latter is what I think gets many of the more experienced reviewers - lots of the patches just solve stuff they don't care about. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
* Jaime Casanova (ja...@2ndquadrant.com) wrote: On Tue, Dec 16, 2014 at 11:32 AM, Robert Haas robertmh...@gmail.com wrote: It has been proposed that we do a general list of people at the bottom of the release notes who helped review during that cycle. That would be less intrusive and possibly a good idea, but would we credit the people who did a TON of reviewing? Everyone who reviewed even one patch? Somewhere in between? Would committers be excluded because we just expect them to help or included because credit is important to established community members too? To what extent would this be duplicative of http://www.postgresql.org/community/contributors/ ? Not much really, I tried to get my name on that list a couple of years ago, when i reviewed more than i do now, and never got it. And while my name is in a couple commit messages, that is a lot harder to show to people... Having your name in a list of other names at the bottom of the release notes page, without any indication of what you helped with, would work better? Perhaps it would but I tend to doubt it. If the release notes were expanded to include more than just a list of reviewer names, and more along the lines of the way patch authors are credited, that would be more valuable for the reviewers but I'm not excited about such an increase in the size of the release notes. To play it out a bit though, we could do something like: * Add Magic PostgreSQL feature (Author: John Doe, Reviewed By: Jane Doe) With the commit history now including that information, generally, this would hopefully not require too much additional work for the release notes authors. As for the 'rules' about who is considered a reviewer for these purposes, I'd suggest it just go off of whatever is mentioned in the commit history but exclude committers (and perhaps major contributors..? they are already acknowledged on the website, after all..). That would help keep the lists of reviewers from getting too large, hopefully. I'm pretty sure this was brought up previously and shot down but perhaps feelings have changed about it now, especially as the information is generally in the commit history and there is already some amount of policy regarding who gets what credit as it's more-or-less up to the committers. I definitely think that, in general, reviews need to be more than it compiles to get that credit. you know, it's kind of frustrating when some not-yet customers ask for certificated engineers, and there isn't any official (as in from community) certificate so you need to prove you're a contributor so let's see this random commit messages... Another thought I had was to suggest we consider *everyone* to be a contributor and implement a way to tie together the mailing list archives with the commit history and perhaps the commitfest app and make it searchable and indexed on some website. eg: contributors.postgresql.org/sfrost - Recent commits - Recent commit mentions - Recent emails to any list - Recent commitfest app activity - Recent wiki page updates ... Ideally with a way for individuals to upload a photo, provide a company link, etc, similar to what the existing Major Contributors have today. Obviously, this is not a small task to develop and there is some risk of abuse (which I expect the other folks on the infra team will point out and likely tar and feather me for suggesting this at all..) but it might be along the same lines as Bruce's PgLife.. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Commitfest problems
On 15/12/14 19:08, Robert Haas wrote: On Sun, Dec 14, 2014 at 2:24 PM, Mark Cave-Ayland mark.cave-ayl...@ilande.co.uk wrote: However if it were posted as part of patchset with a subject of [PATCH] gram.y: add EXCLUDED pseudo-alias to bison grammar then this may pique my interest enough to review the changes to the grammar rules, with the barrier for entry reduced to understanding just the PostgreSQL-specific parts. Meh. Such a patch couldn't possibly compile or work without modifying other parts of the tree. And I'm emphatically opposed to splitting a commit that would have taken the tree from one working state to another working state into a series of patches that would have taken it from a working state through various non-working states and eventually another working state. Furthermore, if you really want to review that specific part of the patch, just look for what changed in gram.y, perhaps by applying the patch and doing git diff src/backend/parser/gram.y. This is really not hard. Okay I agree that the suggested subject above was a little misleading, so let me try and clarify this further. If I were aiming to deliver this as an individual patch as part of a patchset, my target for this particular patch would be to alter both the bison grammar *and* the minimal underlying code/structure changes into a format that compiles but adds no new features. So why is this useful? Because the parser in PostgreSQL is important and people have sweated many hours to ensure its performance is the best it can be. By having a checkpoint with just the basic parser changes then it becomes really easy to bisect performance issues down to just this one particular area, rather than the feature itself. And as per my original message it is a much lower barrier of entry for a potential reviewer who has previous bison experience (and is curious about PostgreSQL) to review the basic rule changes than a complete new feature. I certainly agree that there are cases where patch authors could and should put more work into decomposing large patches into bite-sized chunks. But I don't think that's always possible, and I don't think that, for example, applying BRIN in N separate pieces would have been a step forward. It's one feature, not many. I completely agree with you here. Maybe this isn't exactly the same for PostgreSQL but in general for a new QEMU feature I could expect a patchset of around 12 patches to be posted. Of those 12 patches, probably patches 1-9 are internal API changes, code refactoring and preparation work, patch 10 is a larger patch containing the feature, and patches 11-12 are for tidy-up and removing unused code sections. Even with the best patch review process in the world, there will still be patches that introduce bugs, and the bugs are pretty much guaranteed to be caused by patches 1-9. Imagine a subtle bug in an internal API change which exhibits itself not in the feature added by the patchset itself but in another mostly unrelated part of the system; maybe this could be caused by a pass-by-value API being changed to a pass-by-reference API in patches 1-9 and this tickles a bug due to an unexpected lifecycle heap access elsewhere causing random data to be returned. Being able to bisect this issue down to a single specific patch suddenly becomes very useful indeed. ATB, Mark. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 15/12/14 19:24, Andrew Dunstan wrote: On 12/15/2014 02:08 PM, Robert Haas wrote: On Sun, Dec 14, 2014 at 2:24 PM, Mark Cave-Ayland mark.cave-ayl...@ilande.co.uk wrote: However if it were posted as part of patchset with a subject of [PATCH] gram.y: add EXCLUDED pseudo-alias to bison grammar then this may pique my interest enough to review the changes to the grammar rules, with the barrier for entry reduced to understanding just the PostgreSQL-specific parts. Meh. Such a patch couldn't possibly compile or work without modifying other parts of the tree. And I'm emphatically opposed to splitting a commit that would have taken the tree from one working state to another working state into a series of patches that would have taken it from a working state through various non-working states and eventually another working state. Furthermore, if you really want to review that specific part of the patch, just look for what changed in gram.y, perhaps by applying the patch and doing git diff src/backend/parser/gram.y. This is really not hard. I certainly agree that there are cases where patch authors could and should put more work into decomposing large patches into bite-sized chunks. But I don't think that's always possible, and I don't think that, for example, applying BRIN in N separate pieces would have been a step forward. It's one feature, not many. +1 I have in the past found the bunch of patches approach to be more that somewhat troublesome, especially when one gets here is an update to patch nn of the series and one has to try to compose a coherent set of patches in order to test them. A case in point I recall was the original Foreign Data Wrapper patchset. In practice, people don't tend to post updates to individual patches in that way. What happens is that after a week or so of reviews, people go away and rework the patch and come back with a complete updated patchset clearly marked as [PATCHv2] with the same subject line and an updated cover letter describing the changes, so a complete coherent patchset is always available. Now as I mentioned previously, one of the disadvantages of splitting patches in this way is that mailing list volume tends to grow quite quickly - hence the use of [PATCH] filters and system identifiers in the subject line to help mitigate this. Whilst the volume of mail was a shock to begin with, having stuck with it for a while I personally find that the benefits to development outweigh the costs. Each individual project is different, but I believe that there are good ideas here that can be used to improve workflow, particularly when it comes to patch review. ATB, Mark. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 12/16/2014 05:53 PM, Mark Cave-Ayland wrote: In practice, people don't tend to post updates to individual patches in that way. Exactly. Much like if you push a new revision of a working branch, you repost all the changesets - or should. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 15/12/14 19:27, Robert Haas wrote: On Sun, Dec 14, 2014 at 4:53 PM, Mark Cave-Ayland mark.cave-ayl...@ilande.co.uk wrote: What I find frustrating is that I've come back from a workflow where I've been reviewing/testing patches within months of joining a project because the barrier for entry has been so low, and yet even with much longer experience of the PostgreSQL codebase I feel I can't do the same for patches submitted to the commitfest. And why is this? It's because while I know very specific areas of the code well, many patches span different areas of the source tree of which I have little and/or no experience, which when supplied as a single monolithic patch makes it impossible for me to give meaningful review to all but a tiny proportion of them. So, there are certainly some large patches that do that, and they typically require a very senior reviewer. But there are lots of small patches too, touching little enough that you can learn enough to give them a decent review even if you've never looked at that before. I find myself in that situation as a reviewer and committer pretty regularly; being a committer doesn't magically make you an expert on the entire code base. You can (and I do) focus your effort on the things you know best, but you have to step outside your comfort zone sometimes, or you never learn anything new. Right. Which is why I'm advocating the approach of splitting patches in relevant chunks so that experts in those areas can review them in parallel. And even better, if I then want to start digging into other parts of the system as time and interest allow then I can choose to begin by picking a subject line from a patchset and going through this small individual patch in detail rather than a single large patch. It has often been suggested that people learn best when studying a mix of 80% of things they already know compared to 20% of things they don't. At least with more granular patches people can find a comfortable ratio of new/old material vs. a single large patch which could be 70-80% of completely new material and therefore much more difficult to pick-up. Another thought to ponder here is that by reviewing smaller patches which takes less time, for the same time cost a reviewer with experience in one specific area can in theory review and provide feedback on another 2-3 patchsets which should help relieve patch review starvation across patchset submissions. ATB, Mark. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 15 December 2014 at 19:52, Josh Berkus j...@agliodbs.com wrote: On 12/15/2014 11:27 AM, Robert Haas wrote: I feel like we used to be better at encouraging people to participate in the CF even if they were not experts, and to do the best they can based on what they did know. That was a helpful dynamic. Sure, the reviews weren't perfect, but more people helped, and reviewing some of the patch well and some of it in a more cursory manner is way better than reviewing none of it at all. Well, it was strongly expressed to me by a number of senior contributors on this list and at the developer meeting that inexpert reviews were not really wanted, needed or helpful. As such, I stopped recruiting new reviewers (and, for that matter, doing them myself). I don't know if the same goes for anyone else. I don't remember saying that, hearing it or agreeing with it and I don't agree with it now. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 16/12/14 04:57, Noah Misch wrote: But that doesn't mean we should be turning anyone away. We should not. +1. Some of the best reviews I've seen are ones where the reviewer expressed doubts about the review's quality, so don't let such doubts keep you from participating. Every defect you catch saves a committer time; a review that finds 3 of the 10 defects in a patch is still a big help. Some patch submissions waste the community's time, but it's almost impossible to waste the community's time by posting a patch review. Confusion may have arisen due to statements that we need more expert reviewers, which is also true. (When an expert writes a sufficiently-complex patch, it's important that a second expert examine the patch at some point.) If you're a novice reviewer, your reviews do help to solve that problem by reducing the workload on expert reviewers. I should add that by reducing the barrier of entry for patch review, there is definitely potential to find common errors before a patch gets analysed in detail by a committer. For example I may not know a lot about certain PostgreSQL subsystems but from a decent C coding background I can pick out certain suspicious things in patches, e.g. potential overflows, pointer arithmetic bugs, spelling mistakes/incorrect comments etc. and flag them to the original submitter to double-check. ATB, Mark. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 12/16/14 11:26 AM, Mark Cave-Ayland wrote: On 15/12/14 19:27, Robert Haas wrote: So, there are certainly some large patches that do that, and they typically require a very senior reviewer. But there are lots of small patches too, touching little enough that you can learn enough to give them a decent review even if you've never looked at that before. I find myself in that situation as a reviewer and committer pretty regularly; being a committer doesn't magically make you an expert on the entire code base. You can (and I do) focus your effort on the things you know best, but you have to step outside your comfort zone sometimes, or you never learn anything new. Right. Which is why I'm advocating the approach of splitting patches in relevant chunks so that experts in those areas can review them in parallel. I don't see how splitting patches up would help with that. I often look at only the parts of patches that touch the things I've worked with before. And in doing that, I've found that having the context there is absolutely crucial almost every single time, since I commonly ask myself Why do we need to do this to implement feature X?, and only looking at the rest of the complete patch (or patch set, however you want to think about it) reveals that. Of course, me looking at parts of patches, finding nothing questionable and not sending an email about my findings (or lack thereof) hardly counts as review, so somebody else still has to review the actual patch as a whole. Nor do I get any credit for doing any of that, which might be a show-stopper for someone else. But I think that's just because I'm not doing it correctly. I don't see why someone couldn't comment on a patch saying I've reviewed the grammar changes, and they look good to me. .marko -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 16/12/14 07:33, David Rowley wrote: On 16 December 2014 at 18:18, Josh Berkus j...@agliodbs.com mailto:j...@agliodbs.com wrote: Man. You're equating stuff that's not the same. You didn't get your way (and I'm tentatively on your side onthat one) and take that to imply that we don't want more reviewers. During that thread a couple people said that novice reviewers added no value to the review process, and nobody argued with them then. I've also been told this to my face at pgCon, and when I've tried organizing patch review events. I got the message, which is why I stopped trying to get new reviewers. And frankly: if we're opposed to giving credit to patch reviewers, we're opposed to having them. I'd just like to add something which might be flying below the radar of more senior people. There are people out there (ike me) working on PostgreSQL more for the challenge and perhaps the love of the product, who make absolutely zero money out of it. For these people getting credit where it's due is very important. I'm pretty happy with this at the moment and I can't imagine any situation where not crediting reviewers would be beneficial to anyone. This is exactly where I am at the moment, having previously been more involved with the development side of PostgreSQL during the past. Personally having a credit as a patch reviewer isn't particularly important to me, since mail archives are good enough these days that if people do query my contributions towards projects then I can point them towards any reasonable search engine. The biggest constraint on my ability to contribute is *time*. Imagine the situation as a reviewer that I am currently on the mailing list for two well-known open source projects and I also have a day job and a home life to contend with. For the spare time that I have for review, one of these projects requires me to download attachment(s), apply them to a git tree (hopefully it still applies), run a complete make check regression series, try and analyse a patch which will often reference parts to which I have no understanding, and then write up a coherent email and submit it to the mailing list. Realistically to do all this and provide a review that is going to be of use to a committer is going to take a minimum of 1-2 hours, and even then there's a good chance that I've easily missed obvious bugs in the parts of the system I don't understand well. For the second project, I can skim through my inbox daily picking up specific areas I work on/are interested in, hit reply to add a couple of lines of inline comments to the patch and send feedback to the author/list in just a few minutes. The obvious question is, of course, which project gets the majority share of my spare review time? ATB, Mark. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 16/12/14 10:49, Marko Tiikkaja wrote: On 12/16/14 11:26 AM, Mark Cave-Ayland wrote: On 15/12/14 19:27, Robert Haas wrote: So, there are certainly some large patches that do that, and they typically require a very senior reviewer. But there are lots of small patches too, touching little enough that you can learn enough to give them a decent review even if you've never looked at that before. I find myself in that situation as a reviewer and committer pretty regularly; being a committer doesn't magically make you an expert on the entire code base. You can (and I do) focus your effort on the things you know best, but you have to step outside your comfort zone sometimes, or you never learn anything new. Right. Which is why I'm advocating the approach of splitting patches in relevant chunks so that experts in those areas can review them in parallel. I don't see how splitting patches up would help with that. I often look at only the parts of patches that touch the things I've worked with before. And in doing that, I've found that having the context there is absolutely crucial almost every single time, since I commonly ask myself Why do we need to do this to implement feature X?, and only looking at the rest of the complete patch (or patch set, however you want to think about it) reveals that. I've already covered this earlier in the thread so I won't go into too much detail, but the overall flow of the patchset is described by the cover letter (please feel free to look at the example link I posted). In terms of individual patches within a patchset then if the combination of the cover letter and individual commit message don't give you enough context then the developer needs to fix this; either the patchset has been split at a nonsensical place or either one or other of the cover letter and/or commit message need to be revised. Of course, me looking at parts of patches, finding nothing questionable and not sending an email about my findings (or lack thereof) hardly counts as review, so somebody else still has to review the actual patch as a whole. Nor do I get any credit for doing any of that, which might be a show-stopper for someone else. But I think that's just because I'm not doing it correctly. I don't see why someone couldn't comment on a patch saying I've reviewed the grammar changes, and they look good to me. Sure, there is always scope for doing that which is what splitting patches and constant review encourages. In terms of the current commitfest system, the process for review is clearly documented and as I've pointed out in my response to David, extremely heavyweight in comparison. ATB, Mark. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On Tue, Dec 16, 2014 at 8:09 AM, Mark Cave-Ayland mark.cave-ayl...@ilande.co.uk wrote: For the spare time that I have for review, one of these projects requires me to download attachment(s), apply them to a git tree (hopefully it still applies), run a complete make check regression series, try and analyse a patch which will often reference parts to which I have no understanding, and then write up a coherent email and submit it to the mailing list. Realistically to do all this and provide a review that is going to be of use to a committer is going to take a minimum of 1-2 hours, and even then there's a good chance that I've easily missed obvious bugs in the parts of the system I don't understand well. For the second project, I can skim through my inbox daily picking up specific areas I work on/are interested in, hit reply to add a couple of lines of inline comments to the patch and send feedback to the author/list in just a few minutes. Notice though that on the second project, the review is made in the air. You didn't build, nor ran tests, you're guessing how the code performs rather than seeing it perform, so the review is light compared to the first. Some of the effort of the first review is pointless, but not all of it. Running make check may be a good task for a CI tool, but if you ignore the result of make check, your review is missing an important bit of information to be weighted. There's something to be learned from the open build service ( http://openbuildservice.org/ ), there, review requests contain in the interface the results of the build and rpmlint (it's for rpms). It makes the review easy yet informed. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On Tue, Dec 16, 2014 at 11:09:34AM +, Mark Cave-Ayland wrote: On 16/12/14 07:33, David Rowley wrote: On 16 December 2014 at 18:18, Josh Berkus j...@agliodbs.com mailto:j...@agliodbs.com wrote: Man. You're equating stuff that's not the same. You didn't get your way (and I'm tentatively on your side onthat one) and take that to imply that we don't want more reviewers. During that thread a couple people said that novice reviewers added no value to the review process, and nobody argued with them then. I've also been told this to my face at pgCon, and when I've tried organizing patch review events. I got the message, which is why I stopped trying to get new reviewers. And frankly: if we're opposed to giving credit to patch reviewers, we're opposed to having them. I'd just like to add something which might be flying below the radar of more senior people. There are people out there (ike me) working on PostgreSQL more for the challenge and perhaps the love of the product, who make absolutely zero money out of it. For these people getting credit where it's due is very important. I'm pretty happy with this at the moment and I can't imagine any situation where not crediting reviewers would be beneficial to anyone. This is exactly where I am at the moment, having previously been more involved with the development side of PostgreSQL during the past. Personally having a credit as a patch reviewer isn't particularly important to me, since mail archives are good enough these days that if people do query my contributions towards projects then I can point them towards any reasonable search engine. The biggest constraint on my ability to contribute is *time*. Imagine the situation as a reviewer that I am currently on the mailing list for two well-known open source projects and I also have a day job and a home life to contend with. For the spare time that I have for review, one of these projects requires me to download attachment(s), apply them to a git tree (hopefully it still applies), run a complete make check regression series, try and analyse a patch which will often reference parts to which I have no understanding, and then write up a coherent email and submit it to the mailing list. Realistically to do all this and provide a review that is going to be of use to a committer is going to take a minimum of 1-2 hours, and even then there's a good chance that I've easily missed obvious bugs in the parts of the system I don't understand well. For the second project, I can skim through my inbox daily picking up specific areas I work on/are interested in, hit reply to add a couple of lines of inline comments to the patch and send feedback to the author/list in just a few minutes. With utmost respect, you've missed something really important in the second that the first has, and frankly isn't terribly onerous: does an actual system produce working code? In the PostgreSQL case, you can stop as soon as you discover that the patch doesn't apply to master or that ./configure doesn't work, or that the code doesn't compile: elapsed time = 5 minutes. Or you can keep moving until you have made progress for the time you've allotted. But the bigger issue, as others have pointed out, has never been a technical one. It's motivating people whose time is already much in demand to spend some of it on reviewing. I wasn't discouraged by the preliminary patch review process or any feedback I got. My absence lately has more to do with other demands on my time. Cheers, David. -- David Fetter da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 16/12/14 13:37, Claudio Freire wrote: For the second project, I can skim through my inbox daily picking up specific areas I work on/are interested in, hit reply to add a couple of lines of inline comments to the patch and send feedback to the author/list in just a few minutes. Notice though that on the second project, the review is made in the air. You didn't build, nor ran tests, you're guessing how the code performs rather than seeing it perform, so the review is light compared to the first. I think this doing developers in general a little injustice here. The general rule for a submitting patch to *any* project is: i) does it apply to the current source tree and ii) does it build and pass the regression tests? Maybe there is a greater incidence of this happening in PostgreSQL compared to other projects? But in any case the project has every right to refuse further review if these 2 simple criteria are not met. Also with a submission from git, you can near 100% guarantee that the author has actually built and run the code before submission. I have seen occasions where a patch has been submitted, and a committer will send a quick email along the lines of The patch looks good, but I've just applied a patch that will conflict with this, so please rebase and resubmit. You mention about performance, but again I've seen from this list that good developers can generally pick up on a lot of potential regressions by eye, e.g. lookup times go from O(N) to O(N^2) without having to resort to building and testing the code. And by breaking into small chunks people can focus their time on reviewing the areas they are good at. Occasionally sometimes people do get a patch ready for commit but it fails build/test on one particular platform. In that case, the committer simply posts the build/test failure to the list and requests that the patchset be fixed ready for re-test. This is a very rare occurrence though. Also you mentioned about light review but I wouldn't call it that. I see it more as with each iteration the magnifying glass used to look at the code gets bigger and bigger. A lot of initial comments for the second project are along the lines of this doesn't look right - won't this overflow on values 2G? or you're assuming here that A occurs before B, whereas if you run with -foo this likely won't work. And this is the area which I believe will have the greatest benefit for PostgreSQL - making it easier to catch and rework the obvious flaws in the patch in a timely manner before it gets to the committer. ATB, Mark. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 16/12/14 13:44, David Fetter wrote: On Tue, Dec 16, 2014 at 11:09:34AM +, Mark Cave-Ayland wrote: On 16/12/14 07:33, David Rowley wrote: On 16 December 2014 at 18:18, Josh Berkus j...@agliodbs.com mailto:j...@agliodbs.com wrote: Man. You're equating stuff that's not the same. You didn't get your way (and I'm tentatively on your side onthat one) and take that to imply that we don't want more reviewers. During that thread a couple people said that novice reviewers added no value to the review process, and nobody argued with them then. I've also been told this to my face at pgCon, and when I've tried organizing patch review events. I got the message, which is why I stopped trying to get new reviewers. And frankly: if we're opposed to giving credit to patch reviewers, we're opposed to having them. I'd just like to add something which might be flying below the radar of more senior people. There are people out there (ike me) working on PostgreSQL more for the challenge and perhaps the love of the product, who make absolutely zero money out of it. For these people getting credit where it's due is very important. I'm pretty happy with this at the moment and I can't imagine any situation where not crediting reviewers would be beneficial to anyone. This is exactly where I am at the moment, having previously been more involved with the development side of PostgreSQL during the past. Personally having a credit as a patch reviewer isn't particularly important to me, since mail archives are good enough these days that if people do query my contributions towards projects then I can point them towards any reasonable search engine. The biggest constraint on my ability to contribute is *time*. Imagine the situation as a reviewer that I am currently on the mailing list for two well-known open source projects and I also have a day job and a home life to contend with. For the spare time that I have for review, one of these projects requires me to download attachment(s), apply them to a git tree (hopefully it still applies), run a complete make check regression series, try and analyse a patch which will often reference parts to which I have no understanding, and then write up a coherent email and submit it to the mailing list. Realistically to do all this and provide a review that is going to be of use to a committer is going to take a minimum of 1-2 hours, and even then there's a good chance that I've easily missed obvious bugs in the parts of the system I don't understand well. For the second project, I can skim through my inbox daily picking up specific areas I work on/are interested in, hit reply to add a couple of lines of inline comments to the patch and send feedback to the author/list in just a few minutes. With utmost respect, you've missed something really important in the second that the first has, and frankly isn't terribly onerous: does an actual system produce working code? In the PostgreSQL case, you can stop as soon as you discover that the patch doesn't apply to master or that ./configure doesn't work, or that the code doesn't compile: elapsed time = 5 minutes. Or you can keep moving until you have made progress for the time you've allotted. As per my previous email, it's generally quite rare for a developer to post non-working code to a list (in some cases someone will send a quick reply pointing that this needs to be rebased because it appears to reference an old API). From what I see in PostgreSQL this mostly happens because of bitrot between the time the patch was posted to the list and the actual commitfest itself - so in one way the commitfest system exacerbates this particular problem further. But the bigger issue, as others have pointed out, has never been a technical one. It's motivating people whose time is already much in demand to spend some of it on reviewing. I wasn't discouraged by the preliminary patch review process or any feedback I got. My absence lately has more to do with other demands on my time. I am completely in agreement with you here. My approach is more along the lines of that I know access to long periods of time to review patches is often impractical, and so how can the review process be made more time-efficient in order to allow you, me and other people in similar time-limited environments to be able to participate more? ATB, Mark. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On Tue, Dec 16, 2014 at 11:47 AM, Mark Cave-Ayland mark.cave-ayl...@ilande.co.uk wrote: On 16/12/14 13:37, Claudio Freire wrote: For the second project, I can skim through my inbox daily picking up specific areas I work on/are interested in, hit reply to add a couple of lines of inline comments to the patch and send feedback to the author/list in just a few minutes. Notice though that on the second project, the review is made in the air. You didn't build, nor ran tests, you're guessing how the code performs rather than seeing it perform, so the review is light compared to the first. I think this doing developers in general a little injustice here. The general rule for a submitting patch to *any* project is: i) does it apply to the current source tree and ii) does it build and pass the regression tests? That it's a policy doesn't guarantee that people submitting patches will adhere. They could be failing to adhere even unknowingly (ie: bitrot - there's quite some time going on from first submission to first review). Maybe there is a greater incidence of this happening in PostgreSQL compared to other projects? Perhaps, but it's because the reviewers take care to test things before they hit the build farm. That basic testing really is something for a CI tool, like the build farm itself, not reviewers. But you cannot wait until after comitting to let the build farm tell you something's broken: you need CI results *during* review. But in any case the project has every right to refuse further review if these 2 simple criteria are not met. Nobody said otherwise Also with a submission from git, you can near 100% guarantee that the author has actually built and run the code before submission. I don't see how. Forks don't have travis ci unless you add it, or am I mistaken? You mention about performance, but again I've seen from this list that good developers can generally pick up on a lot of potential regressions by eye, e.g. lookup times go from O(N) to O(N^2) without having to resort to building and testing the code. And by breaking into small chunks people can focus their time on reviewing the areas they are good at. I meant performance as in running, not really performance. Sorry for the confusion. I meant, you're looking at the code and guessing how it runs, but you don't really know. It's easy to make assumptions that are false while reviewing, quickly disproved by actually running tests. A light review without ever building can fail to detect those issues. A heavier review patching up and building manually is too much manual work that could really be automated. The optimum is somewhere between those two extremes. Occasionally sometimes people do get a patch ready for commit but it fails build/test on one particular platform. Again, pre-review CI can take care of this. In that case, the committer simply posts the build/test failure to the list and requests that the patchset be fixed ready for re-test. This is a very rare occurrence though. It shouldn't need to reach the repo, don't you think? Also you mentioned about light review but I wouldn't call it that. I've made my fair share of light reviews (not for pg though) and can recognize the description. It is a light review what you described. Surely not all reviews with inline comments are light reviews, but what you described was indeed a light review. A lot of initial comments for the second project are along the lines of this doesn't look right - won't this overflow on values 2G? or you're assuming here that A occurs before B, whereas if you run with -foo this likely won't work. And this is the area which I believe will have the greatest benefit for PostgreSQL - making it easier to catch and rework the obvious flaws in the patch in a timely manner before it gets to the committer. If you read carefully my reply, I'm not at all opposed to that. I'm just pointing out, that easier reviews need not result in better reviews. Better, easier reviews are those where the trivial reviews are automated, as is done in the project I linked. Formatting, whether it still applies, and whether it builds and checks pass, all those are automatable. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 16/12/14 15:42, Claudio Freire wrote: Also with a submission from git, you can near 100% guarantee that the author has actually built and run the code before submission. I don't see how. Forks don't have travis ci unless you add it, or am I mistaken? Well as I mentioned in my last email, practically all developers will rebase and run make check on their patched tree before submitting to the list. Hopefully there aren't hordes of developers deliberating creating and submitting broken patches ;) You mention about performance, but again I've seen from this list that good developers can generally pick up on a lot of potential regressions by eye, e.g. lookup times go from O(N) to O(N^2) without having to resort to building and testing the code. And by breaking into small chunks people can focus their time on reviewing the areas they are good at. I meant performance as in running, not really performance. Sorry for the confusion. Okay no worries. I meant, you're looking at the code and guessing how it runs, but you don't really know. It's easy to make assumptions that are false while reviewing, quickly disproved by actually running tests. A light review without ever building can fail to detect those issues. A heavier review patching up and building manually is too much manual work that could really be automated. The optimum is somewhere between those two extremes. Correct. My analogy here was that people with varying abilities review the patches at their experience level, and once low-hanging/obvious design issues have been resolved then more experienced developers will start to review the patch at a deeper level. Occasionally sometimes people do get a patch ready for commit but it fails build/test on one particular platform. Again, pre-review CI can take care of this. Yes - see my next reply... In that case, the committer simply posts the build/test failure to the list and requests that the patchset be fixed ready for re-test. This is a very rare occurrence though. It shouldn't need to reach the repo, don't you think? When I say repo, I mean the local repo of the tree maintainer. Currently the tree maintainer pulls each merge request into his local tree, performs a buildfarm test and only pushes the merge upstream once this has passed. I guess the PostgreSQL equivalent of this would be having a merge-pending branch on the buildfarm rather than just a post-commit build of git master (which we see from reports to the list periodically fails in this way) and only pushing a set of patches when the buildfarm comes back green. Also you mentioned about light review but I wouldn't call it that. I've made my fair share of light reviews (not for pg though) and can recognize the description. It is a light review what you described. Surely not all reviews with inline comments are light reviews, but what you described was indeed a light review. A lot of initial comments for the second project are along the lines of this doesn't look right - won't this overflow on values 2G? or you're assuming here that A occurs before B, whereas if you run with -foo this likely won't work. And this is the area which I believe will have the greatest benefit for PostgreSQL - making it easier to catch and rework the obvious flaws in the patch in a timely manner before it gets to the committer. If you read carefully my reply, I'm not at all opposed to that. I'm just pointing out, that easier reviews need not result in better reviews. Better, easier reviews are those where the trivial reviews are automated, as is done in the project I linked. Yes I can definitely agree with that. See below again: Formatting, whether it still applies, and whether it builds and checks pass, all those are automatable. I should add that QEMU provides a branch of checkpatch.pl in the source tree which submitters are requested to run on their patchset before submission to the list. This catches patches that don't meet the project style guidelines including casing, braces, trailing whitespace, tab/space issues etc. and that's before the patch is even submitted to the list. ATB, Mark. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On Tue, Dec 16, 2014 at 2:33 AM, David Rowley dgrowle...@gmail.com wrote: I'd just like to add something which might be flying below the radar of more senior people. There are people out there (ike me) working on PostgreSQL more for the challenge and perhaps the love of the product, who make absolutely zero money out of it. For these people getting credit where it's due is very important. I'm pretty happy with this at the moment and I can't imagine any situation where not crediting reviewers would be beneficial to anyone. We routinely and regularly contribute reviews in the commit logs for precisely this reason. I don't think anyone is opposed to that. There is some opposition to crediting them in the release notes because the one time Bruce tried it made for an enormous volume of additional text in the release notes, and there were cases where people's names were mentioned on relatively equal footing when their contributions were very much unequal. For example, let's take a look at the commit message for Hot Standby: Simon Riggs, with significant and lengthy review by Heikki Linnakangas, including streamlined redesign of snapshot creation and two-phase commit. Important contributions from Florian Pflug, Mark Kirkwood, Merlin Moncure, Greg Stark, Gianni Ciolli, Gabriele Bartolini, Hannu Krosing, Robert Haas, Tatsuo Ishii, Hiroyuki Yamada plus support and feedback from many other community members. The release note ended up looking like this: Allow a standby server to accept read-only queries (Simon Riggs, Heikki Linnakangas) Including all of the other names of people who made important contributions, many of which consisted of reviewing, would make that release note item - and many others - really, really long, so I'm not in favor of that. Crediting reviewers is important, but so is having the release notes be readable. It has been proposed that we do a general list of people at the bottom of the release notes who helped review during that cycle. That would be less intrusive and possibly a good idea, but would we credit the people who did a TON of reviewing? Everyone who reviewed even one patch? Somewhere in between? Would committers be excluded because we just expect them to help or included because credit is important to established community members too? To what extent would this be duplicative of http://www.postgresql.org/community/contributors/ ? I'm not necessarily averse to doing something here, but the reason why nothing has happened has much more to do with the fact that it's hard to figure out exactly what the best thing would be than any idea that we don't want to credit reviewers. We do want to credit reviewers, AND WE DO, as a quick look at 'git log' will speedily reveal. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On Tue, Dec 16, 2014 at 12:18 AM, Josh Berkus j...@agliodbs.com wrote: On 12/15/2014 07:34 PM, Andres Freund wrote: On 2014-12-15 16:14:30 -0800, Josh Berkus wrote: Read the thread on this list where I suggested crediting reviewers in the release notes. Man. You're equating stuff that's not the same. You didn't get your way (and I'm tentatively on your side onthat one) and take that to imply that we don't want more reviewers. During that thread a couple people said that novice reviewers added no value to the review process, and nobody argued with them then. I've also been told this to my face at pgCon, and when I've tried organizing patch review events. I got the message, which is why I stopped trying to get new reviewers. I think what was said is that it isn't very useful to have reviewers who just report that the patch applies and passes make check. But any review that does any study of the code, or finds a bug in the functionality, or reports that the patch does NOT apply and/or pass make check, or suggests a way that the documentation could be improved, or fixes a typo in a comment, or diagnoses a whitespace error is useful. Summarizing that as novice reviewers added no value to the review process is like summarizing the plot of Superman as alien invades earth. And frankly: if we're opposed to giving credit to patch reviewers, we're opposed to having them. This is also an incredibly misleading summary of what the real issue was, as I just said in my previous email. We do credit reviewers, routinely, and have for years. We have not reached an agreement on whether or exactly how to credit them in the release notes. You may think that there's no value in having your name show up in a commit log message and that the release notes are the only thing that counts, but I don't think everyone feels that way. I *still* get a kick out of it every time somebody types my name into one of those messages. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 12/16/2014 4:32 AM, Simon Riggs wrote: On 15 December 2014 at 19:52, Josh Berkus j...@agliodbs.com wrote: On 12/15/2014 11:27 AM, Robert Haas wrote: I feel like we used to be better at encouraging people to participate in the CF even if they were not experts, and to do the best they can based on what they did know. That was a helpful dynamic. Sure, the reviews weren't perfect, but more people helped, and reviewing some of the patch well and some of it in a more cursory manner is way better than reviewing none of it at all. Well, it was strongly expressed to me by a number of senior contributors on this list and at the developer meeting that inexpert reviews were not really wanted, needed or helpful. As such, I stopped recruiting new reviewers (and, for that matter, doing them myself). I don't know if the same goes for anyone else. I don't remember saying that, hearing it or agreeing with it and I don't agree with it now. As a reviewer from long long ago, I can tell you that I wasn't sure I was even helpful. Things got busy at work, and I may not have been useful, and I annoyed some people on pg-general, so I walked away for a while. I have no knowledge of community-building so this might be a bad idea: Perhaps levels (or titles) of reviewer's would be helpful. A freshman reviewer is not expected to do anything useful, is expected to make mistakes, and is there to learn. The community votes and promotes them to junior (or whatever). They know they are on the right track. A junior review is useful but maybe not as complete as a senior reviewer. Maybe I'll aspire to work harder, and maybe not, but at least I know what I'm doing is useful. If I never get promoted, then I know, as well. Freshmen know its ok to make mistakes. They know who they can contact for help. I think I like belts better (yellow, green, red, black). I think this gives me two things: 1) permission to mess up 2) ability to measure myself -Andy -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On Tue, Dec 16, 2014 at 10:15 AM, Mark Cave-Ayland mark.cave-ayl...@ilande.co.uk wrote: Well as I mentioned in my last email, practically all developers will rebase and run make check on their patched tree before submitting to the list. Even when this is true, and with people new to the project submitting patches I'm not sure it can be assumed, time passes and things can change between submission and review. I've seen a fair number of Needs rebase comments on patches, through no fault of the original submitter.
Re: [HACKERS] Commitfest problems
On 12/16/2014 08:48 AM, Mike Blackwell wrote: On Tue, Dec 16, 2014 at 10:15 AM, Mark Cave-Ayland mark.cave-ayl...@ilande.co.uk mailto:mark.cave-ayl...@ilande.co.ukwrote: Well as I mentioned in my last email, practically all developers will rebase and run make check on their patched tree before submitting to the list. Even when this is true, and with people new to the project submitting patches I'm not sure it can be assumed, time passes and things can change between submission and review. I've seen a fair number of Needs rebase comments on patches, through no fault of the original submitter. This really should be taken care of by automation. Magnus's new system will be a significant step forwards on enabling that. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
* Craig Ringer (cr...@2ndquadrant.com) wrote: It's not like development on a patch series is difficult. You commit small fixes and changes, then you 'git rebase -i' and reorder them to apply to the appropriate changesets. Or you can do a 'rebase -i' and in 'e'dit mode make amendments to individual commits. Or you can commit 'fixup!'s that get auto-squashed. I don't have a problem with this, but it's an independent consideration for how a patch might be developed vs. what the main git repo looks like. I don't think it makes sense to commit to the main repo a new catalog table without any commands to manage it, nor to have a catalog table which can be managed through the grammar but doesn't actually do anything due to lacking the backend code for it. Now, I fully agree that we want to continue to build features on top of other features, but, in general, those need to be independently valuable features when they are committed to the main repo (eg: WITH CHECK for security barrier views went in first as it was independently useful, and then RLS simply built on top of it). This is part of my grumbling about the use of git like it's still CVS. Our git repo is actually readable and reasonably easy to follow and, for my part, that's a great thing we have that most other projects don't. That isn't to say that we shouldn't develop with smaller pieces, but I tend to agree with Tom that it's actually easier (for me, at least) to review a single, complete, patch than a bunch of independent ones which build on each other. Perhaps that's my failing or a fault of my processes, but I will actually sit and read through a patch in my email client quite often. In general, I expect the code to compile and pass 'make check', those are necessary, of course, but detecting compile-time problems is something the compiler is good at and I'm not. Thinking about the impact of a new data structure, a given code block, or making sure that all of the pieces of a new catalog row are set correctly are things that the compiler isn't good at and is where a reviewer is most valuable. Pulling the code in and testing it by hand is useful, as is getting into gdb and looking at the structures as they are manipulated, but I find it extremely important to also actually review the *code*, which means reading it and considering are there places this patch needs to be touching that it isn't? how will this other bit of code react to these changes? does this code still look sane and like one person thought through the whole code path? do the comments explain why things are happening or do they just repeat what the code already says?, etc. This goes back to the excellent point elsewhere on this thread that the current documentation for reviewers is far too focused on the mechanical bits which could basically be automated (in fact, I think Peter's already got most of it automated..). Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Commitfest problems
David, * David Rowley (dgrowle...@gmail.com) wrote: I'd just like to add something which might be flying below the radar of more senior people. There are people out there (ike me) working on PostgreSQL more for the challenge and perhaps the love of the product, who make absolutely zero money out of it. For these people getting credit where it's due is very important. I'm pretty happy with this at the moment and I can't imagine any situation where not crediting reviewers would be beneficial to anyone. Thanks for this. One question which has been brought up in the past is the specific level of credit. That you're happy with the credit you've been given thus far is great as it happens to support the side that I'm on. :D However, could you quantify what, exactly, you feel is approrpiate credit for reviewers and authors..? I'll intentionally omit the options that have been presented in the past to try and avoid influencing your response. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Commitfest problems
On Wed, Dec 17, 2014 at 12:03 AM, Stephen Frost sfr...@snowman.net wrote: David, * David Rowley (dgrowle...@gmail.com) wrote: I'd just like to add something which might be flying below the radar of more senior people. There are people out there (ike me) working on PostgreSQL more for the challenge and perhaps the love of the product, who make absolutely zero money out of it. For these people getting credit where it's due is very important. I'm pretty happy with this at the moment and I can't imagine any situation where not crediting reviewers would be beneficial to anyone. Thanks for this. One question which has been brought up in the past is the specific level of credit. That you're happy with the credit you've been given thus far is great as it happens to support the side that I'm on. :D However, could you quantify what, exactly, you feel is approrpiate credit for reviewers and authors..? I'll intentionally omit the options that have been presented in the past to try and avoid influencing your response. Mentioning them in list of contributors, for one.
Re: [HACKERS] Commitfest problems
* Robert Haas (robertmh...@gmail.com) wrote: Including all of the other names of people who made important contributions, many of which consisted of reviewing, would make that release note item - and many others - really, really long, so I'm not in favor of that. Crediting reviewers is important, but so is having the release notes be readable. Agreed. It has been proposed that we do a general list of people at the bottom of the release notes who helped review during that cycle. That would be less intrusive and possibly a good idea, but would we credit the people who did a TON of reviewing? Everyone who reviewed even one patch? Somewhere in between? Would committers be excluded because we just expect them to help or included because credit is important to established community members too? To what extent would this be duplicative of http://www.postgresql.org/community/contributors/ ? I don't particularly like this idea. I'm not necessarily averse to doing something here, but the reason why nothing has happened has much more to do with the fact that it's hard to figure out exactly what the best thing would be than any idea that we don't want to credit reviewers. We do want to credit reviewers, AND WE DO, as a quick look at 'git log' will speedily reveal. Agreed. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Commitfest problems
On 12/16/2014 01:38 PM, Stephen Frost wrote: * Robert Haas (robertmh...@gmail.com) wrote: Including all of the other names of people who made important contributions, many of which consisted of reviewing, would make that release note item - and many others - really, really long, so I'm not in favor of that. Crediting reviewers is important, but so is having the release notes be readable. Agreed. It has been proposed that we do a general list of people at the bottom of the release notes who helped review during that cycle. That would be less intrusive and possibly a good idea, but would we credit the people who did a TON of reviewing? Everyone who reviewed even one patch? Somewhere in between? Would committers be excluded because we just expect them to help or included because credit is important to established community members too? To what extent would this be duplicative of http://www.postgresql.org/community/contributors/ ? I don't particularly like this idea. I do. I think it's an emminently sensible idea that gives credit without disturbing the readability of the release notes. As for where we draw the line, I would rather me more than less inclusive. Anyone who gets a review credit in the git log should be mentioned. I don't care that much whether or not committers are mentioned. I'm not necessarily averse to doing something here, but the reason why nothing has happened has much more to do with the fact that it's hard to figure out exactly what the best thing would be than any idea that we don't want to credit reviewers. We do want to credit reviewers, AND WE DO, as a quick look at 'git log' will speedily reveal. Agreed. I can't believe how much we are tying ourselves up in knots over this. It's not a good sign. Surely we trust the committers and the preparers of the release notes to use some judgement, once we agree on general guidelines. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
* Andrew Dunstan (and...@dunslane.net) wrote: On 12/16/2014 01:38 PM, Stephen Frost wrote: * Robert Haas (robertmh...@gmail.com) wrote: It has been proposed that we do a general list of people at the bottom of the release notes who helped review during that cycle. That would be less intrusive and possibly a good idea, but would we credit the people who did a TON of reviewing? Everyone who reviewed even one patch? Somewhere in between? Would committers be excluded because we just expect them to help or included because credit is important to established community members too? To what extent would this be duplicative of http://www.postgresql.org/community/contributors/ ? I don't particularly like this idea. I do. I think it's an emminently sensible idea that gives credit without disturbing the readability of the release notes. So, when I was first getting started with PG however many years ago, I was ecstatic to see my name show up in a commit message. Hugely increasing our release notes to include a bunch of names all shoved together without any indication of what was done by each individual doesn't feel, to me at least, as likely to change that feeling in either direction. On the flip side, I would be strongly against *not* including authors and reviewers in the commit messages, regardless of some big list in the release notes. Basically, I see the value of giving credit in the commit history and the mailing lists as huge while having a long list of names in the release notes really isn't valuable. I'm not necessarily averse to doing something here, but the reason why nothing has happened has much more to do with the fact that it's hard to figure out exactly what the best thing would be than any idea that we don't want to credit reviewers. We do want to credit reviewers, AND WE DO, as a quick look at 'git log' will speedily reveal. Agreed. I can't believe how much we are tying ourselves up in knots over this. It's not a good sign. Surely we trust the committers and the preparers of the release notes to use some judgement, once we agree on general guidelines. I agree with this. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Commitfest problems
This whole conversation reminds me of an interview I just read: https://opensource.com/business/14/12/interview-jono-bacon-xprize-director-community -- Command Prompt, Inc. - http://www.commandprompt.com/ 503-667-4564 PostgreSQL Support, Training, Professional Services and Development High Availability, Oracle Conversion, @cmdpromptinc If we send our children to Caesar for their education, we should not be surprised when they come back as Romans. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
Stephen Frost sfr...@snowman.net writes: So, when I was first getting started with PG however many years ago, I was ecstatic to see my name show up in a commit message. Hugely increasing our release notes to include a bunch of names all shoved together without any indication of what was done by each individual doesn't feel, to me at least, as likely to change that feeling in either direction. On the flip side, I would be strongly against *not* including authors and reviewers in the commit messages, regardless of some big list in the release notes. Basically, I see the value of giving credit in the commit history and the mailing lists as huge while having a long list of names in the release notes really isn't valuable. We'd have to continue the practice of crediting people in individual commit messages in any case, because the commit log is the raw material from which the release notes are made. I have no strong feelings either way about whether we should change the current practice of crediting authors but not reviewers in the release notes. I don't feel that it's broken as-is, but I'm open to change if enough people want to. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On Tue, Dec 16, 2014 at 11:32 AM, Robert Haas robertmh...@gmail.com wrote: It has been proposed that we do a general list of people at the bottom of the release notes who helped review during that cycle. That would be less intrusive and possibly a good idea, but would we credit the people who did a TON of reviewing? Everyone who reviewed even one patch? Somewhere in between? Would committers be excluded because we just expect them to help or included because credit is important to established community members too? To what extent would this be duplicative of http://www.postgresql.org/community/contributors/ ? Not much really, I tried to get my name on that list a couple of years ago, when i reviewed more than i do now, and never got it. And while my name is in a couple commit messages, that is a lot harder to show to people... you know, it's kind of frustrating when some not-yet customers ask for certificated engineers, and there isn't any official (as in from community) certificate so you need to prove you're a contributor so let's see this random commit messages... -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación Phone: +593 4 5107566 Cell: +593 987171157 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On Sun, Dec 14, 2014 at 05:21:06PM +, Mark Cave-Ayland wrote: I should add here that the QEMU folk do tend to go to great lengths to preserve bisectability; often intermediate compatibility APIs are introduced early in the patchset and then removed at the very end when the final feature is implemented. I agree with Tom on this, and I want to point out that certain software projects benefit more from modularized development than others , e.g. QEMU is an interface library and therefore probably does things in a more modular way than usual. For example, they are probably not adding new SQL commands or configuration settings in the same way Postgres does. It would be interesting to compare the directory span of a typical Postgres patch vs. a QEMU or Linux kernel one. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 2014-12-15 11:21:03 -0500, Bruce Momjian wrote: On Sun, Dec 14, 2014 at 05:21:06PM +, Mark Cave-Ayland wrote: I should add here that the QEMU folk do tend to go to great lengths to preserve bisectability; often intermediate compatibility APIs are introduced early in the patchset and then removed at the very end when the final feature is implemented. I agree with Tom on this, and I want to point out that certain software projects benefit more from modularized development than others , e.g. QEMU is an interface library and therefore probably does things in a more modular way than usual. For example, they are probably not adding new SQL commands or configuration settings in the same way Postgres does. I'm not following. What do you mean with 'interface library'? I'm pretty sure qemu very regularly adds features including configuration settings/parameters. It would be interesting to compare the directory span of a typical Postgres patch vs. a QEMU or Linux kernel one. I don't believe this really is a question of the type of project. I think it's more that especially the kernel has had to deal with similar problems at a much larger scale. And the granular approach somewhat works for them. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 15/12/14 16:28, Andres Freund wrote: I don't believe this really is a question of the type of project. I think it's more that especially the kernel has had to deal with similar problems at a much larger scale. And the granular approach somewhat works for them. Correct. My argument was that dividing patches into smaller, more reviewable chunks lessens the barrier for reviewers since many people can review the individual patches as appropriate to their area of expertise. The benefits of this are that the many parts of the patchset get reviewed early by a number of people, which reduces the workload on the core developers as they only need to focus on a small number of individual patches. Hence patches get reworked and merged much more quickly in this way. This is in contrast to the commitfest system where a single patch is i) often held until the next commitfest (where bitrot often sets in) and ii) requires the reviewer to have good knowledge all of the areas covered by the patch in order to give a meaningful review, which significantly reduces the pool of available reviewers. ATB, Mark. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On Sat, Dec 13, 2014 at 1:37 AM, Craig Ringer cr...@2ndquadrant.com wrote: On 12/12/2014 06:02 AM, Josh Berkus wrote: Speaking as the originator of commitfests, they were *always* intended to be a temporary measure, a step on the way to something else like continuous integration. I'd really like to see the project revisit some of the underlying assumptions that're being made in this discussion: - Patches must be email attachments to a mailing list - Changes must be committed by applying a diff ... and take a look at some of the options a git-based workflow might offer, especially in combination with some of the tools out there that help track working branches, run CI, etc. Having grown used to push/pull workflows with CI integration I find the PostgreSQL patch workflow very frustrating, especially for larger patches. It's particularly annoying to see a patch series squashed into a monster patch whenever it changes hands or gets rebased, because it's being handed around as a great honking diff not a git working branch. Is it time to stop using git like CVS? Perhaps it is just my inexperience with it, but I find the way that mediawiki, for example, uses git to be utterly baffling. The git log is bloated with the same change being listed multiple times, at least once as a commit and again as a merge. If your suggestion would be to go in that direction, I don't think that would be an improvement. Cheers, Jeff
Re: [HACKERS] Commitfest problems
On Mon, Dec 15, 2014 at 11:52 AM, Josh Berkus j...@agliodbs.com wrote: On 12/15/2014 11:27 AM, Robert Haas wrote: I feel like we used to be better at encouraging people to participate in the CF even if they were not experts, and to do the best they can based on what they did know. That was a helpful dynamic. Sure, the reviews weren't perfect, but more people helped, and reviewing some of the patch well and some of it in a more cursory manner is way better than reviewing none of it at all. Well, it was strongly expressed to me by a number of senior contributors on this list and at the developer meeting that inexpert reviews were not really wanted, needed or helpful. As such, I stopped recruiting new reviewers (and, for that matter, doing them myself). I don't know if the same goes for anyone else. Really? I thought we were pretty consistent in encouraging new reviewers. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 12/15/2014 12:05 PM, Peter Geoghegan wrote: On Mon, Dec 15, 2014 at 11:52 AM, Josh Berkus j...@agliodbs.com wrote: On 12/15/2014 11:27 AM, Robert Haas wrote: I feel like we used to be better at encouraging people to participate in the CF even if they were not experts, and to do the best they can based on what they did know. That was a helpful dynamic. Sure, the reviews weren't perfect, but more people helped, and reviewing some of the patch well and some of it in a more cursory manner is way better than reviewing none of it at all. Well, it was strongly expressed to me by a number of senior contributors on this list and at the developer meeting that inexpert reviews were not really wanted, needed or helpful. As such, I stopped recruiting new reviewers (and, for that matter, doing them myself). I don't know if the same goes for anyone else. Really? I thought we were pretty consistent in encouraging new reviewers. Read the thread on this list where I suggested crediting reviewers in the release notes. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 2014-12-15 16:14:30 -0800, Josh Berkus wrote: Read the thread on this list where I suggested crediting reviewers in the release notes. Man. You're equating stuff that's not the same. You didn't get your way (and I'm tentatively on your side onthat one) and take that to imply that we don't want more reviewers. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On Mon, Dec 15, 2014 at 03:29:19PM -0500, Andrew Dunstan wrote: On 12/15/2014 03:16 PM, Andres Freund wrote: On 2014-12-15 11:52:35 -0800, Josh Berkus wrote: On 12/15/2014 11:27 AM, Robert Haas wrote: I feel like we used to be better at encouraging people to participate in the CF even if they were not experts, and to do the best they can based on what they did know. That was a helpful dynamic. Sure, the reviews weren't perfect, but more people helped, and reviewing some of the patch well and some of it in a more cursory manner is way better than reviewing none of it at all. Well, it was strongly expressed to me by a number of senior contributors on this list and at the developer meeting that inexpert reviews were not really wanted, needed or helpful. I think that's pretty far away from what was said. I welcome reviews at all levels, both as a developer and as a committer. It is true that we are very short on reviewers with in depth knowledge and experience, and this is the real problem we have far more than any technological issues people might have. But that doesn't mean we should be turning anyone away. We should not. +1. Some of the best reviews I've seen are ones where the reviewer expressed doubts about the review's quality, so don't let such doubts keep you from participating. Every defect you catch saves a committer time; a review that finds 3 of the 10 defects in a patch is still a big help. Some patch submissions waste the community's time, but it's almost impossible to waste the community's time by posting a patch review. Confusion may have arisen due to statements that we need more expert reviewers, which is also true. (When an expert writes a sufficiently-complex patch, it's important that a second expert examine the patch at some point.) If you're a novice reviewer, your reviews do help to solve that problem by reducing the workload on expert reviewers. Thanks, nm -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 12/15/2014 07:34 PM, Andres Freund wrote: On 2014-12-15 16:14:30 -0800, Josh Berkus wrote: Read the thread on this list where I suggested crediting reviewers in the release notes. Man. You're equating stuff that's not the same. You didn't get your way (and I'm tentatively on your side onthat one) and take that to imply that we don't want more reviewers. During that thread a couple people said that novice reviewers added no value to the review process, and nobody argued with them then. I've also been told this to my face at pgCon, and when I've tried organizing patch review events. I got the message, which is why I stopped trying to get new reviewers. And frankly: if we're opposed to giving credit to patch reviewers, we're opposed to having them. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 2014-12-15 21:18:40 -0800, Josh Berkus wrote: On 12/15/2014 07:34 PM, Andres Freund wrote: On 2014-12-15 16:14:30 -0800, Josh Berkus wrote: Read the thread on this list where I suggested crediting reviewers in the release notes. Man. You're equating stuff that's not the same. You didn't get your way (and I'm tentatively on your side onthat one) and take that to imply that we don't want more reviewers. During that thread a couple people said that novice reviewers added no value to the review process, and nobody argued with them then. I've also been told this to my face at pgCon, and when I've tried organizing patch review events. I got the message, which is why I stopped trying to get new reviewers. I think there's a very large difference in what novice reviewers do. A schematic 'in context format, compiles and survives make check' type of test indeed doesn't seem to be particularly useful to me. A novice reviewer that tries out the feature by reading the docs noticing shortages there on the way, and then verifies that the feature works outside of the two regression tests added is something entirely different. Novice reviewers *can* review the code quality as well - it's just that many we had didn't. I think the big problem is that a good review takes time - and that's what many of the novice reviewers I've observed weren't really aware of. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 16 Dec 2014 7:43 am, Andres Freund and...@2ndquadrant.com wrote: On 2014-12-15 21:18:40 -0800, Josh Berkus wrote: On 12/15/2014 07:34 PM, Andres Freund wrote: On 2014-12-15 16:14:30 -0800, Josh Berkus wrote: Read the thread on this list where I suggested crediting reviewers in the release notes. Man. You're equating stuff that's not the same. You didn't get your way (and I'm tentatively on your side onthat one) and take that to imply that we don't want more reviewers. During that thread a couple people said that novice reviewers added no value to the review process, and nobody argued with them then. I've also been told this to my face at pgCon, and when I've tried organizing patch review events. I got the message, which is why I stopped trying to get new reviewers. I think there's a very large difference in what novice reviewers do. A schematic 'in context format, compiles and survives make check' type of test indeed doesn't seem to be particularly useful to me. A novice reviewer that tries out the feature by reading the docs noticing shortages there on the way, and then verifies that the feature works outside of the two regression tests added is something entirely different. Novice reviewers *can* review the code quality as well - it's just that many we had didn't. I think the big problem is that a good review takes time - and that's what many of the novice reviewers I've observed weren't really aware of. The review docs also over-emphasise the mechanical parts of review around make check etc, which may make it seem like that alone is quite useful. When really it's just the beginning. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 16 December 2014 at 18:18, Josh Berkus j...@agliodbs.com wrote: Man. You're equating stuff that's not the same. You didn't get your way (and I'm tentatively on your side onthat one) and take that to imply that we don't want more reviewers. During that thread a couple people said that novice reviewers added no value to the review process, and nobody argued with them then. I've also been told this to my face at pgCon, and when I've tried organizing patch review events. I got the message, which is why I stopped trying to get new reviewers. And frankly: if we're opposed to giving credit to patch reviewers, we're opposed to having them. I'd just like to add something which might be flying below the radar of more senior people. There are people out there (ike me) working on PostgreSQL more for the challenge and perhaps the love of the product, who make absolutely zero money out of it. For these people getting credit where it's due is very important. I'm pretty happy with this at the moment and I can't imagine any situation where not crediting reviewers would be beneficial to anyone. Regards David Rowley
Re: [HACKERS] Commitfest problems
On 12/16/2014 03:08 AM, Robert Haas wrote: On Sun, Dec 14, 2014 at 2:24 PM, Mark Cave-Ayland mark.cave-ayl...@ilande.co.uk wrote: However if it were posted as part of patchset with a subject of [PATCH] gram.y: add EXCLUDED pseudo-alias to bison grammar then this may pique my interest enough to review the changes to the grammar rules, with the barrier for entry reduced to understanding just the PostgreSQL-specific parts. Meh. Such a patch couldn't possibly compile or work without modifying other parts of the tree. And I'm emphatically opposed to splitting a commit that would have taken the tree from one working state to another working state into a series of patches that would have taken it from a working state through various non-working states and eventually another working state. Absolutely. I think that'd be awful. I'm a fan of fairly fine grained patch series, but only where each patch makes some sense by its self and doesn't break the tree. Splitting stuff up purely for the sake of it or having patches that are non-working intermediate states is painful and pointless. Occasoinally it can be worth having a patch that introduces intermediate compatibility code that's removed later, especially when it's small or very self-contained. Most of the time I'm inclined to think that's not worth it and it can create annoying noise in the history. I'm only advocating it where the individual parts make a reasonable amount of sense standing alone, and actually work by themselves. Anyway, I'm not contributing much to the real topic here, which is commitfest process issues. It's probably getting toward time for me to butt out. Furthermore, if you really want to review that specific part of the patch, just look for what changed in gram.y, perhaps by applying the patch and doing git diff src/backend/parser/gram.y. This is really not hard. Yup ... and with 'git am' and then 'git diff' on subtrees, it's pretty convenient. It's even easier when someone pushes work to GitHub working trees, so you don't have to mess about with applying the changes, but that's a trivial and unimportant convenience. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 13/12/14 09:37, Craig Ringer wrote: Speaking as the originator of commitfests, they were *always* intended to be a temporary measure, a step on the way to something else like continuous integration. I'd really like to see the project revisit some of the underlying assumptions that're being made in this discussion: - Patches must be email attachments to a mailing list - Changes must be committed by applying a diff ... and take a look at some of the options a git-based workflow might offer, especially in combination with some of the tools out there that help track working branches, run CI, etc. Having grown used to push/pull workflows with CI integration I find the PostgreSQL patch workflow very frustrating, especially for larger patches. It's particularly annoying to see a patch series squashed into a monster patch whenever it changes hands or gets rebased, because it's being handed around as a great honking diff not a git working branch. Is it time to stop using git like CVS? While not so active these days with PostgreSQL, I believe there is still great scope for improving the workflow of reviewing and applying patches. And while the commitfests were a good idea when they started, it's obvious that in their current form they do not scale to meet the current rate of development. If I could name just one thing that I think would improve things it would be submission of patches to the list in git format-patch format. Why? Because it enables two things: 1) by definition patches are small-chunked into individually reviewable pieces and 2) subject lines allow list members to filter things that aren't relevant to them. Here's an example of how the workflow works for the QEMU project which I'm involved in, which also has similar issues in terms of numbers of developers and supported platforms: 1) Developer submits a patch to the -devel list with git-format-patch - For subsystems with listed maintainers in the MAINTAINERS file, the maintainer must be CCd on the patchset (this is so that even if developers decide to filter PATCH emails to the list, they still get the CC copy) - Patchsets must be iterative and fully bisectable, with clear comments supplied in the commit message - Any patchset with 1 patch MUST have a cover letter - All patches have a Signed-off-by indicating that the developer has accepted the terms of the project license - Each subject line should be in the format [PATCH] subsystem: one line comment to enabled people to determine its relevance 2) Other developers start reviewing patches There are several mini-workflows that tend to occur here so I'll try and give some popular examples. - If a patch is a build fix, one of the core committers will verify and apply directly to the master repository - If a maintainer is happy with the whole patchset, they reply to the cover letter with a Reviewed-by and a line stating which subtree the patchset has been applied to. Maintainers add a Signed-off-by to all patches accepted via their subtree. - A maintainer may indicate that the patch itself is fine, but the commit message is not clear/incorrect and should be changed before being resubmitted - Any member of the mailing list may reply/review an individual patch by hitting reply in their mail client. Patch comments are included inline. If a reviewer is happy with an individual patch, they reply to the patch and add a Reviewed-by line; anyone can provide a review, even if they are not a maintainer - For complex patches, a maintainer may request that the patchset may be split into further patchsets. In general patchsets of 20-30 patches tend to be frowned upon, and will often immediately result in a reply saying please break this down into smaller chunks - A maintainer may reply and say that while the patchset in its final form needs more work, some clean-ups introduced by the patch are ready to be applied, e.g. please resubmit patches 1-4, 7 and 8 as a separate patchset which can then be applied directly to a subtree 3) Patchset tidy-up and submission After the initial review, new versions of the patchset are generally reposted to the list within a few days. - The patch version is included in the header with the same subject line, e.g. [PATCHv2] Add new feature - The cover letter contains a mini-changelog giving the changes between v1 and v2 e.g. v2: Fix spelling mistake pointed out by Peter Change lock ordering requested by Andreas - Reviewed-by tags for individual patches sent to the list are appended to the commit message for that patch before resubmission Once a maintainer has accepted a patch into their subtree, they send a pull request to the core maintainers to apply their trees to the main repository. I appreciate that currently merge commits aren't used in the PostgreSQL git repository so a pull request effectively becomes rebase this patchset onto master and push. So why does this help patch review? From my experience I can
Re: [HACKERS] Commitfest problems
-BEGIN PGP SIGNED MESSAGE- Hash: RIPEMD160 #2 is solved by my previous comments about giving the CFM/C the authority. -Core could do that, they are in charge of release. I don't think authority is the solution. Or certainly not one that would work with an open source project like ours. What *would* work is to identify and fix the friction points that prevent people from joining, make the work harder than it needs to be, and makes people stop reviewing? I could quickly identify a handful of things, primarily among them the awful link-to-the-archives to gather up all the patches process. We have git, let's use it as it was intended. - -- Greg Sabino Mullane g...@turnstep.com End Point Corporation http://www.endpoint.com/ PGP Key: 0x14964AC8 201412141011 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -BEGIN PGP SIGNATURE- iEYEAREDAAYFAlSNqK8ACgkQvJuQZxSWSsiewwCffAxv8xSZEyLWFz/b2+PxXOXS xB4An2ubr7ovELtFMKZOZCsFHQAyVca4 =S6ZQ -END PGP SIGNATURE- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 12/14/2014 10:35 PM, Mark Cave-Ayland wrote: Compare this to say, for example, huge patches such as RLS. I specifically objected to that being flattened into a single monster patch when I saw that'd been done. If you look at my part in the work on the row security patch, while I was ultimately unsuccessful in getting the patch mergeable I spent quite a bit of time splitting it up into a logical patch-series for sane review and development. I am quite annoyed that it was simply flattened back into an unreviewable, hard-to-follow blob and committed in that form. It's not like development on a patch series is difficult. You commit small fixes and changes, then you 'git rebase -i' and reorder them to apply to the appropriate changesets. Or you can do a 'rebase -i' and in 'e'dit mode make amendments to individual commits. Or you can commit 'fixup!'s that get auto-squashed. This is part of my grumbling about the use of git like it's still CVS. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 12/14/2014 10:35 PM, Mark Cave-Ayland wrote: If I could name just one thing that I think would improve things it would be submission of patches to the list in git format-patch format. Why? Because it enables two things: 1) by definition patches are small-chunked into individually reviewable pieces and 2) subject lines allow list members to filter things that aren't relevant to them. Personally I do just that. Even though the official docs say context diff is preferred, I think most people now use context diff in practice, from 'git diff'. I'm surprised most people don't use git format-patch . Note that we can't just git am a patch from git format-patch at the moment, because policy is that the committer should be the Author field. The project would have to define a transition point where we started using the git Author field for the author and the separate Committer field, like git-am does by default. I'm not sure that'll ever actually happen, or that it's even desirable. - Patchsets must be iterative and fully bisectable, with clear comments supplied in the commit message Fully support this one. Also in the smallest reasonable size divisions. - If a maintainer is happy with the whole patchset, they reply to the cover letter with a Reviewed-by and a line stating which subtree the patchset has been applied to. Maintainers add a Signed-off-by to all patches accepted via their subtree. Sounds a lot like the kernel workflow (though in practice it seems like the kernel folks tend bypass all this mailing list guff and just use direct pulls/pushes for lots of things). - Any member of the mailing list may reply/review an individual patch by hitting reply in their mail client. Patch comments are included inline. If a reviewer is happy with an individual patch, they reply to the patch and add a Reviewed-by line; anyone can provide a review, even if they are not a maintainer This is fun with many mail clients that like to linewrap text bodies and don't permit inline replies to attached text. 6) Smaller patches are applied more often By breaking large patches down into smaller chunks, even if the main feature itself isn't ready to be applied to the main repository then a lot of the preceding patches laying down the groundwork can. I think PostgreSQL does OK on smaller patches - at least sometimes. There can be a great deal of bikeshedding and endless arguments, back-and-forth about utility, in-tree users, etc, but no formal process is ever going to change that. The process of getting an uncontroversial patch into Pg is generally painless, if often rather slow unless a committer sees it and commits it immediately upon it being posted. The benefits here are that people with large out-of-tree patches I'm not sure we have all that many people in that category - though I'm a part of a team that most certainly does fit the bill. Even the small patches for BDR have been challenging and often contentious though. Since as large features get implemented as a series of smaller patches A big barrier here is the we must have in-tree users thing, which makes it hard to do iterative work on a finer grained scale. Some C-level unit tests that let us exercise small units of functionality might ease those complaints. Right now the closest we have to that is contrib/test_[blah] which is only useful for public API and even then quite limited. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 14/12/14 15:51, Craig Ringer wrote: On 12/14/2014 10:35 PM, Mark Cave-Ayland wrote: Compare this to say, for example, huge patches such as RLS. I specifically objected to that being flattened into a single monster patch when I saw that'd been done. If you look at my part in the work on the row security patch, while I was ultimately unsuccessful in getting the patch mergeable I spent quite a bit of time splitting it up into a logical patch-series for sane review and development. I am quite annoyed that it was simply flattened back into an unreviewable, hard-to-follow blob and committed in that form. It's not like development on a patch series is difficult. You commit small fixes and changes, then you 'git rebase -i' and reorder them to apply to the appropriate changesets. Or you can do a 'rebase -i' and in 'e'dit mode make amendments to individual commits. Or you can commit 'fixup!'s that get auto-squashed. This is part of my grumbling about the use of git like it's still CVS. I just want to add that I'm always grateful for the time that yourself and all committers put into PostgreSQL, and my example of RLS was really to signify big feature X rather than to pick on a particular aspect of that patch. I apologise if you though I was in any way criticising the work that you, or anyone else, put into this feature. The argument I wanted to make is that if someone starts seeing a problem with a current build of PostgreSQL and git bisects it down to a particular commit, then smaller, iterative commits are extremely helpful in this regard. When searching for a needle in a haystack, there is very big difference between a add big feature X haystack and a change struct alignment for Y haystack, the latter which is implicit in having smaller, iterative patchsets. ATB, Mark. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
Craig Ringer cr...@2ndquadrant.com writes: On 12/14/2014 10:35 PM, Mark Cave-Ayland wrote: Compare this to say, for example, huge patches such as RLS. I specifically objected to that being flattened into a single monster patch when I saw that'd been done. If you look at my part in the work on the row security patch, while I was ultimately unsuccessful in getting the patch mergeable I spent quite a bit of time splitting it up into a logical patch-series for sane review and development. I am quite annoyed that it was simply flattened back into an unreviewable, hard-to-follow blob and committed in that form. TBH, I'm not really on board with this line of argument. I don't find broken-down patches to be particularly useful for review purposes. An example I was just fooling with this week is the GROUPING SETS patch, which was broken into three sections for no good reason at all. (The fourth and fifth subpatches, being alternative solutions to one problem, are in a different category of course.) Too often, decisions made in one subpatch don't make any sense until you see the larger picture. Also, speaking of the larger picture: the current Postgres revision history amounts to 37578 commits (as of sometime yesterday) --- and that's just in the HEAD branch. If we'd made an effort to break feature patches into bite-size chunks like you're recommending here, we'd probably have easily half a million commits in the mainline history. That would not be convenient to work with, and I really doubt that it would be more useful for git bisect purposes, and I'll bet a large amount of money that most of them would not have had commit messages composed with any care at all. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 14/12/14 15:57, Craig Ringer wrote: On 12/14/2014 10:35 PM, Mark Cave-Ayland wrote: If I could name just one thing that I think would improve things it would be submission of patches to the list in git format-patch format. Why? Because it enables two things: 1) by definition patches are small-chunked into individually reviewable pieces and 2) subject lines allow list members to filter things that aren't relevant to them. Personally I do just that. Even though the official docs say context diff is preferred, I think most people now use context diff in practice, from 'git diff'. I'm surprised most people don't use git format-patch . Note that we can't just git am a patch from git format-patch at the moment, because policy is that the committer should be the Author field. The project would have to define a transition point where we started using the git Author field for the author and the separate Committer field, like git-am does by default. I'm not sure that'll ever actually happen, or that it's even desirable. Sure, I appreciate that. The part I particularly wanted to emphasise here was the use of a system identifier as part of the subject line, e.g. say if there was a hypothetical patchset which sped up PostgreSQL by 20% then you could see subjects like this: [PATCH 1/x] lmgr: reduce lock strength for LWLock [PATCH 2/x] wal: change lock ordering for WAL writes [PATCH 3/x] optimiser: exit early if lock unobtainable While these are obviously unrealistic, what I hope here is that people with interest in these areas would take note of individual patches even if they aren't interested in the entire patchset. So maybe since Andreas did some recent locking work, he spots lmgr in the subject and then makes a note to review that part of the patch. Similary Heikki could spot wal and make a note to look at that, whilst Tom would zero in on the optimiser changes. The aim here is that no one person needs to sit and initially review a complete patch before returning feedback to the developer. - Patchsets must be iterative and fully bisectable, with clear comments supplied in the commit message Fully support this one. Also in the smallest reasonable size divisions. I should add here that the QEMU folk do tend to go to great lengths to preserve bisectability; often intermediate compatibility APIs are introduced early in the patchset and then removed at the very end when the final feature is implemented. - If a maintainer is happy with the whole patchset, they reply to the cover letter with a Reviewed-by and a line stating which subtree the patchset has been applied to. Maintainers add a Signed-off-by to all patches accepted via their subtree. Sounds a lot like the kernel workflow (though in practice it seems like the kernel folks tend bypass all this mailing list guff and just use direct pulls/pushes for lots of things). Yes indeed - mainly from the people on the KVM module side. Again I'm not saying that this workflow is correct for PostgreSQL, I was trying to use this an example to explain *why* the workflow is done in this manner and how it helps developers such as myself. - Any member of the mailing list may reply/review an individual patch by hitting reply in their mail client. Patch comments are included inline. If a reviewer is happy with an individual patch, they reply to the patch and add a Reviewed-by line; anyone can provide a review, even if they are not a maintainer This is fun with many mail clients that like to linewrap text bodies and don't permit inline replies to attached text. Wow really? I can't say I've seen this in practice for a long time but I defer to your experience here. 6) Smaller patches are applied more often By breaking large patches down into smaller chunks, even if the main feature itself isn't ready to be applied to the main repository then a lot of the preceding patches laying down the groundwork can. I think PostgreSQL does OK on smaller patches - at least sometimes. There can be a great deal of bikeshedding and endless arguments, back-and-forth about utility, in-tree users, etc, but no formal process is ever going to change that. The process of getting an uncontroversial patch into Pg is generally painless, if often rather slow unless a committer sees it and commits it immediately upon it being posted. I agree that trivial patches do tend to get picked up reasonably quickly. It strikes me that it may prevent patches slipping through the list if someone were officially nominated as a trivial patch monitor, i.e. someone for developers to ping if their patch has been ignored but it sounds like this is not such an issue in practice. The benefits here are that people with large out-of-tree patches I'm not sure we have all that many people in that category - though I'm a part of a team that most certainly does fit the bill. Even the small patches for BDR have been challenging and often contentious
Re: [HACKERS] Commitfest problems
On 12/14/2014 12:05 PM, Tom Lane wrote: Craig Ringer cr...@2ndquadrant.com writes: On 12/14/2014 10:35 PM, Mark Cave-Ayland wrote: Compare this to say, for example, huge patches such as RLS. I specifically objected to that being flattened into a single monster patch when I saw that'd been done. If you look at my part in the work on the row security patch, while I was ultimately unsuccessful in getting the patch mergeable I spent quite a bit of time splitting it up into a logical patch-series for sane review and development. I am quite annoyed that it was simply flattened back into an unreviewable, hard-to-follow blob and committed in that form. TBH, I'm not really on board with this line of argument. I don't find broken-down patches to be particularly useful for review purposes. An example I was just fooling with this week is the GROUPING SETS patch, which was broken into three sections for no good reason at all. (The fourth and fifth subpatches, being alternative solutions to one problem, are in a different category of course.) Too often, decisions made in one subpatch don't make any sense until you see the larger picture. Also, speaking of the larger picture: the current Postgres revision history amounts to 37578 commits (as of sometime yesterday) --- and that's just in the HEAD branch. If we'd made an effort to break feature patches into bite-size chunks like you're recommending here, we'd probably have easily half a million commits in the mainline history. That would not be convenient to work with, and I really doubt that it would be more useful for git bisect purposes, and I'll bet a large amount of money that most of them would not have had commit messages composed with any care at all. I have tried to stay away from this thread, but ... I'm also quite dubious about this suggested workflow, partly for the reasons Tom gives, and partly because it would constrain the way I work. I tend to commit with little notes to myself in the commit logs, notes that are never intended to become part of the public project history. I should be quite sad to lose that. As for using git bisect, usually when I do this each iteration is quite expensive. Multiplying the number of commits by a factor between 10 and 100, which is what I think this would involve, would just make git bisect have to do between 3 and 7 more iterations, ISTM. That's not a win. On the larger issue, let me just note that I don't believe we have what is fundamentally a technological problem, and while technological changes can of course sometimes make things easier, they can also blind us to the more basic problems we are facing. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 14/12/14 17:05, Tom Lane wrote: Craig Ringer cr...@2ndquadrant.com writes: On 12/14/2014 10:35 PM, Mark Cave-Ayland wrote: Compare this to say, for example, huge patches such as RLS. I specifically objected to that being flattened into a single monster patch when I saw that'd been done. If you look at my part in the work on the row security patch, while I was ultimately unsuccessful in getting the patch mergeable I spent quite a bit of time splitting it up into a logical patch-series for sane review and development. I am quite annoyed that it was simply flattened back into an unreviewable, hard-to-follow blob and committed in that form. TBH, I'm not really on board with this line of argument. I don't find broken-down patches to be particularly useful for review purposes. An example I was just fooling with this week is the GROUPING SETS patch, which was broken into three sections for no good reason at all. (The fourth and fifth subpatches, being alternative solutions to one problem, are in a different category of course.) Too often, decisions made in one subpatch don't make any sense until you see the larger picture. The way in which this is normally handled is via the cover letter which is going to be nicely captured in the mail archives; typically a cover letter for a patchset consists of several paragraphs along the lines of patches 1-3 do some re-org work, patch 4 reworks the Y API for new feature X implemented in patches 9-12. As an example take a look at the cover letter for this patch submitted over the past few days: http://lists.gnu.org/archive/html/qemu-devel/2014-12/msg01990.html. It seems to me that this would give the level of detail which you are looking for. I agree that definitely in some cases patches could be broken into too fine pieces, but then I think it's perfectly okay for committers to turn around and request the series be re-submitted in more sensible chunks if they feel things have gone too far the other way. Also, speaking of the larger picture: the current Postgres revision history amounts to 37578 commits (as of sometime yesterday) --- and that's just in the HEAD branch. If we'd made an effort to break feature patches into bite-size chunks like you're recommending here, we'd probably have easily half a million commits in the mainline history. That would not be convenient to work with, and I really doubt that it would be more useful for git bisect purposes, and I'll bet a large amount of money that most of them would not have had commit messages composed with any care at all. Having worked on a few kernel patches myself, git is surprisingly effective on huge repositories once the cache is warmed up so I don't feel this would be an issue with a PostgreSQL git repository which will be many magnitudes smaller. In terms of commit messages, I don't know if you missed that part of my original response to Craig but it's considered normal for a maintainer to reject a patch because of an incorrect/irrelevant commit message. So if I submitted a patch that fixed a particular bug but you as a committer weren't happy with the commit message, then I'm perfectly okay with you asking me to resubmit with updated/corrected patch message wording. ATB, Mark. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On Sun, Dec 14, 2014 at 9:05 AM, Tom Lane t...@sss.pgh.pa.us wrote: TBH, I'm not really on board with this line of argument. I don't find broken-down patches to be particularly useful for review purposes. An example I was just fooling with this week is the GROUPING SETS patch, which was broken into three sections for no good reason at all. (The fourth and fifth subpatches, being alternative solutions to one problem, are in a different category of course.) Too often, decisions made in one subpatch don't make any sense until you see the larger picture. It sounds like they didn't use the technique effectively, then. I think it will be useful to reviewers that I've broken out the mechanism through which the ON CONFLICT UPDATE patch accepts the EXCLUDED.* pseudo-alias into a separate commit (plus docs in a separate commit, as well as tests) - it's a non-trivial additional piece of code that it wouldn't be outrageous to omit in a release, and isn't particularly anticipated by earlier cumulative commits. Maybe people don't have a good enough sense of what sort of subdivision is appropriate yet. I think that better style will emerge over time. Of course, if you still prefer a monolithic commit, it's pretty easy to produce one from a patch series. It's not easy to go in the other direction, though. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 14/12/14 17:30, Andrew Dunstan wrote: On 12/14/2014 12:05 PM, Tom Lane wrote: Craig Ringer cr...@2ndquadrant.com writes: On 12/14/2014 10:35 PM, Mark Cave-Ayland wrote: Compare this to say, for example, huge patches such as RLS. I specifically objected to that being flattened into a single monster patch when I saw that'd been done. If you look at my part in the work on the row security patch, while I was ultimately unsuccessful in getting the patch mergeable I spent quite a bit of time splitting it up into a logical patch-series for sane review and development. I am quite annoyed that it was simply flattened back into an unreviewable, hard-to-follow blob and committed in that form. TBH, I'm not really on board with this line of argument. I don't find broken-down patches to be particularly useful for review purposes. An example I was just fooling with this week is the GROUPING SETS patch, which was broken into three sections for no good reason at all. (The fourth and fifth subpatches, being alternative solutions to one problem, are in a different category of course.) Too often, decisions made in one subpatch don't make any sense until you see the larger picture. Also, speaking of the larger picture: the current Postgres revision history amounts to 37578 commits (as of sometime yesterday) --- and that's just in the HEAD branch. If we'd made an effort to break feature patches into bite-size chunks like you're recommending here, we'd probably have easily half a million commits in the mainline history. That would not be convenient to work with, and I really doubt that it would be more useful for git bisect purposes, and I'll bet a large amount of money that most of them would not have had commit messages composed with any care at all. I have tried to stay away from this thread, but ... I'm also quite dubious about this suggested workflow, partly for the reasons Tom gives, and partly because it would constrain the way I work. I tend to commit with little notes to myself in the commit logs, notes that are never intended to become part of the public project history. I should be quite sad to lose that. Interestingly enough, I tend to work in a very similar way to this. When submitting patches upstream, I tend to rebase on a new branch and then squash/rework as required. One thing I do tend to find is that once I start rebasing the patches for upstream, I find that many of my personal notes can be rewritten as part of the patch comment (I would like to think that comments useful to myself will be useful to other developers some day). Once the rebase is complete, often I find that I have no non-public comments left so that problem becomes almost non-existent. As for using git bisect, usually when I do this each iteration is quite expensive. Multiplying the number of commits by a factor between 10 and 100, which is what I think this would involve, would just make git bisect have to do between 3 and 7 more iterations, ISTM. That's not a win. I find that this isn't too bad in practice. If you think about monolithic patches during a commitfest, you can imagine that most of them will touch one of the core .h files which will require most things to be rebuilt once applied during bisection. With smaller iterative patchsets, each patch tends to only touch a handful of files and so make install generally only needs to rebuild and link a very small number of files which is reasonably quick. Since all of the changes for global header files are contained in 1-2 patches then the number of complete rebuilds isn't as many as you might expect. On the larger issue, let me just note that I don't believe we have what is fundamentally a technological problem, and while technological changes can of course sometimes make things easier, they can also blind us to the more basic problems we are facing. Absolutely. I firmly believe that the right tools for the job are available, it's really trying to find a workflow solution that works well for everyone involved in the project. ATB, Mark. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 14/12/14 18:24, Peter Geoghegan wrote: On Sun, Dec 14, 2014 at 9:05 AM, Tom Lane t...@sss.pgh.pa.us wrote: TBH, I'm not really on board with this line of argument. I don't find broken-down patches to be particularly useful for review purposes. An example I was just fooling with this week is the GROUPING SETS patch, which was broken into three sections for no good reason at all. (The fourth and fifth subpatches, being alternative solutions to one problem, are in a different category of course.) Too often, decisions made in one subpatch don't make any sense until you see the larger picture. It sounds like they didn't use the technique effectively, then. I think it will be useful to reviewers that I've broken out the mechanism through which the ON CONFLICT UPDATE patch accepts the EXCLUDED.* pseudo-alias into a separate commit (plus docs in a separate commit, as well as tests) - it's a non-trivial additional piece of code that it wouldn't be outrageous to omit in a release, and isn't particularly anticipated by earlier cumulative commits. Maybe people don't have a good enough sense of what sort of subdivision is appropriate yet. I think that better style will emerge over time. For me this is a great example of how to get more developers involved in patch review. Imagine that I'm an experienced developer with little previous exposure to PostgreSQL, but with a really strong flex/bison background. If someone posts a patch to the list as a single grouping sets patch, then I'm going to look at that and think I have no way of understanding this and do nothing with it. However if it were posted as part of patchset with a subject of [PATCH] gram.y: add EXCLUDED pseudo-alias to bison grammar then this may pique my interest enough to review the changes to the grammar rules, with the barrier for entry reduced to understanding just the PostgreSQL-specific parts. What should happen over time is that developers like this would earn the trust of the committers, so committers can spend more time reviewing the remaining parts of the patchset. And of course the project has now engaged a new developer into the community who otherwise may not have not participated. Of course, if you still prefer a monolithic commit, it's pretty easy to produce one from a patch series. It's not easy to go in the other direction, though. Agreed. ATB, Mark. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 12/15/2014 02:46 AM, Mark Cave-Ayland wrote: Interestingly enough, I tend to work in a very similar way to this. When submitting patches upstream, I tend to rebase on a new branch and then squash/rework as required. Same here, and I find it works really well ... when I do it properly. I usually have a private development branch that's full of fixup! commits, WIPs, awful commit messages, etc. Then I have the tree that's been rebased, squashed, and tided up. I periodically tidy my latest work and replace the old clean tree with it, then start my new development branch on top of the new clean tree. This gives me a somewhat nice looking, well ordered patch series to work on top of, while retaining the flexibility to do lots of quick fixes etc. Admittedly, sometimes the development tree gets a bit large and it takes a while before I give it a proper cleanup. My current project being very much in that category. Still - it works well in general. I find that this isn't too bad in practice. If you think about monolithic patches during a commitfest, you can imagine that most of them will touch one of the core .h files which will require most things to be rebuilt once applied during bisection. It's not build time, it's test-run time. Especially if you can't automate the test, or it isn't practical to. That's a legitimate concern - but I don't think we'd see a blowout of patch counts to quite the same degree. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 14/12/14 20:07, Craig Ringer wrote: On 12/15/2014 02:46 AM, Mark Cave-Ayland wrote: Interestingly enough, I tend to work in a very similar way to this. When submitting patches upstream, I tend to rebase on a new branch and then squash/rework as required. Same here, and I find it works really well ... when I do it properly. I usually have a private development branch that's full of fixup! commits, WIPs, awful commit messages, etc. Then I have the tree that's been rebased, squashed, and tided up. I periodically tidy my latest work and replace the old clean tree with it, then start my new development branch on top of the new clean tree. This gives me a somewhat nice looking, well ordered patch series to work on top of, while retaining the flexibility to do lots of quick fixes etc. Admittedly, sometimes the development tree gets a bit large and it takes a while before I give it a proper cleanup. My current project being very much in that category. Still - it works well in general. That describes my workflow fairly well too. I find that this isn't too bad in practice. If you think about monolithic patches during a commitfest, you can imagine that most of them will touch one of the core .h files which will require most things to be rebuilt once applied during bisection. It's not build time, it's test-run time. Especially if you can't automate the test, or it isn't practical to. That's a legitimate concern - but I don't think we'd see a blowout of patch counts to quite the same degree. At the end of the day, each project finds its own level as to how complex individual patches should be and what should comprise a patchset. Over the past couple of years the QEMU process has evolved into its current form as maintainers and developers have figured out what works best for them, and I don't see that PostgreSQL would be any different in this respect - it takes time to adapt to a new workflow and get it right. Having worked on various parts for PostGIS for around 10 years, I've had my head stuck into various parts of the GiST code and have got to know a few parts of it really well. What I find frustrating is that I've come back from a workflow where I've been reviewing/testing patches within months of joining a project because the barrier for entry has been so low, and yet even with much longer experience of the PostgreSQL codebase I feel I can't do the same for patches submitted to the commitfest. And why is this? It's because while I know very specific areas of the code well, many patches span different areas of the source tree of which I have little and/or no experience, which when supplied as a single monolithic patch makes it impossible for me to give meaningful review to all but a tiny proportion of them. I believe that this is one of the main reasons why people struggle to find patch reviewers, with the consequence being that the majority of work falls back onto the CF manager and the committers which is why we have the current situation. ATB, Mark. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 12/12/2014 06:02 AM, Josh Berkus wrote: Speaking as the originator of commitfests, they were *always* intended to be a temporary measure, a step on the way to something else like continuous integration. I'd really like to see the project revisit some of the underlying assumptions that're being made in this discussion: - Patches must be email attachments to a mailing list - Changes must be committed by applying a diff ... and take a look at some of the options a git-based workflow might offer, especially in combination with some of the tools out there that help track working branches, run CI, etc. Having grown used to push/pull workflows with CI integration I find the PostgreSQL patch workflow very frustrating, especially for larger patches. It's particularly annoying to see a patch series squashed into a monster patch whenever it changes hands or gets rebased, because it's being handed around as a great honking diff not a git working branch. Is it time to stop using git like CVS? (/hides) -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 12/12/2014 06:01 AM, David G Johnston wrote: The patch list concept should be formalized, and should include a targeted release concept. IMO, the patch list concept should be discarded in favour of a working tree list. At this point, given the challenges the CF process faces, I can't say I'd be sad if Pg adopted GitHub's push/pull process on their infrastructure ... but I'm aware that's not going to happen, and I understand at least some of the reasons why. I'd be really happy to see the CF somewhat closer to that than a bunch of message-id copy-and-pasting and patch emailing though. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 12/12/14 20:43, Josh Berkus wrote: On 12/12/2014 11:35 AM, Alvaro Herrera wrote: Uh, really? Last I looked at the numbers from SPI treasurer reports, they are not impressive enough to hire a full-time engineer, let alone a senior one. The Linux Foundation has managed to pay for Linus Torvalds somehow, so it does sound possible. We have a number of companies making money all over the globe, at least. I think Álvaro (Herrera) got to the real point when he suggested to fund a developer. Or three, as was also suggested. But to really nail it down, I'd say not only for CFM. I think, overall, the PostgreSQL community would really need to seriously consider raising funds and pay with that full time development for some senior developers. That would also allow to have development more oriented into what the community really wants, rather than what other companies or individuals work for (which is absolutely great, of course). You're looking at this wrong. We have that amount of money in the account based on zero fundraising whatsoever, which we don't do because we don't spend the money. We get roughly $20,000 per year just by putting up a donate link, and not even promoting it. So, what this would take is: 1) a candidate who is currently a known major committer I think it would be even better to sell this approach as a long-term strategy, not tied to any particular candidate. Sure, some known major committer is for sure a good selling point; but a well communicated strategy for a long-term foundation-like fund raising to improve PostgreSQL in certain ways is the way to go. 2) clear goals for what this person would spend their time doing +1. That may be the Core Team based on feedback/input from all the list, or something like that 3) buy-in from the Core Team, the committers, and the general hackers community (including buy-in to the idea of favorable publicity for funding supporters) +1 4) an organizing committee with the time to deal with managing foundation funds +1. Absolutely necessary, otherwise funding will not work If we had those four things, the fundraising part would be easy. I speak as someone who used to raise $600,000 per year for a non-profit in individual gifts alone. I know it sounds difficult, and surely it is, but I believe the PostgreSQL community should be able to raise, globally, some millions per year to stably, and permanently, fund this community-guided development and have our best developers devoted 100% to PostgreSQL. Regards, Álvaro -- Álvaro Hernández Tortosa --- 8Kdata -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On Thu, Dec 11, 2014 at 5:55 PM, Tom Lane t...@sss.pgh.pa.us wrote: Josh Berkus j...@agliodbs.com writes: How about *you* run the next one, Tom? I think the limited amount of time I can put into a commitfest is better spent on reviewing patches than on managing the process. That's not really the point. The point is that managing the last CommitFest in particular is roughly equivalent to having your arm boiled in hot oil. A certain percentage of the people whose patches are obviously not ready to commit complain and moan about how (a) their patch really is ready for prime-time, despite all appearances to the contrary, and/or (b) their patch is so important that it deserves and exception, and/or (c) how you are a real jerk for treating them so unfairly. This is not fun, which is why I've given up on doing it. I could not get a single person to support me when I tried to enforce any scheduling discipline, so my conclusion was that the community did not care about hitting the schedule; and it took weeks of 24x7 effort to build a consensus to reject even one large, problematic patch whose author wasn't willing to admit defeat. If the community is prepared to invest some trusted individuals with real authority, then we might be able to remove some of the pain here, but when that was discussed at a PGCon developer meeting a few years back, it was clear that no more than 20% of the people in the room were prepared to support that concept. At this point, though, I'm not sure how much revisiting that discussion would help. I think the problem we need to solve here is that there are just not enough senior people with an adequate amount of time to review. Whether it's because the patches are more complex or that there are more of them or that those senior people have become less available due to other commitments, we still need more senior people involved to be able to handle the patches we've got in a timely fashion without unduly compromising stability. And we also need to do a better job recruiting and retaining mid-level reviewers, both because that's where senior people eventually come from, and because it reduces the load on the senior people we've already got. (I note that the proposal to have the CFM review everything is merely one way of meeting the need to have senior people spend more time reviewing. But I assure all of you that I spend as much time reviewing as I can find time for. If someone wants to pay me the same salary I'm making now to do nothing but review patches, I'll think about it. But even then, that would also mean that I wasn't spending time writing patches of my own.) -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
Robert Haas wrote: (I note that the proposal to have the CFM review everything is merely one way of meeting the need to have senior people spend more time reviewing. But I assure all of you that I spend as much time reviewing as I can find time for. If someone wants to pay me the same salary I'm making now to do nothing but review patches, I'll think about it. But even then, that would also mean that I wasn't spending time writing patches of my own.) I have heard the idea of a cross-company PostgreSQL foundation of some sort that would hire a developer just to manage commitfests, do patch reviews, apply bugfixes, etc, without the obligations that come from individual companies' schedules for particular development roadmaps, customer support, and the like. Of course, only a senior person would be able to fill this role because it requires considerable experience. Probably this person should be allowed to work on their own patches if they so desire; otherwise there is a risk that experience dilutes. Also, no single company should dictate what this person's priorities are, other than general guidelines: general stability, submitted patches get attention, bugs get closed, releases get out, coffee gets brewed. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On Fri, Dec 12, 2014 at 9:15 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Robert Haas wrote: (I note that the proposal to have the CFM review everything is merely one way of meeting the need to have senior people spend more time reviewing. But I assure all of you that I spend as much time reviewing as I can find time for. If someone wants to pay me the same salary I'm making now to do nothing but review patches, I'll think about it. But even then, that would also mean that I wasn't spending time writing patches of my own.) I have heard the idea of a cross-company PostgreSQL foundation of some sort that would hire a developer just to manage commitfests, do patch reviews, apply bugfixes, etc, without the obligations that come from individual companies' schedules for particular development roadmaps, customer support, and the like. Of course, only a senior person would be able to fill this role because it requires considerable experience. Probably this person should be allowed to work on their own patches if they so desire; otherwise there is a risk that experience dilutes. Also, no single company should dictate what this person's priorities are, other than general guidelines: general stability, submitted patches get attention, bugs get closed, releases get out, coffee gets brewed. Yeah, that would be great, and even better if we could get 2 or 3 positions funded so that the success or failure isn't too much tied to a single individual. But even getting 1 position funded in a stable-enough fashion that someone would be willing to bet on it seems like a challenge. (Maybe other people here are less risk-averse than I am.) -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On Thu, Dec 11, 2014 at 05:55:56PM -0500, Tom Lane wrote: Josh Berkus j...@agliodbs.com writes: How about *you* run the next one, Tom? I think the limited amount of time I can put into a commitfest is better spent on reviewing patches than on managing the process. With utmost respect, Tom, you seem to carve off an enormous amount of time to follow -bugs and -general. What say you unsubscribe to those lists for the duration of your tenure as CFM? Cheers, David. -- David Fetter da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 2014-12-12 07:10:40 -0800, David Fetter wrote: On Thu, Dec 11, 2014 at 05:55:56PM -0500, Tom Lane wrote: Josh Berkus j...@agliodbs.com writes: How about *you* run the next one, Tom? I think the limited amount of time I can put into a commitfest is better spent on reviewing patches than on managing the process. With utmost respect, FWIW, the way you frequently use this phrase doesn't come over as actually being respectful. Tom, you seem to carve off an enormous amount of time to follow -bugs and -general. What say you unsubscribe to those lists for the duration of your tenure as CFM? And why on earth would that be a good idea? These bugs need to be fixed - we're actually behind on that front. Are we now really trying to dictate how other developers manage their time? It's one thing to make up rules that say one review for one commit or something, it's something entirely else to try to assign tasks to them. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
Robert Haas robertmh...@gmail.com writes: On Fri, Dec 12, 2014 at 9:15 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Robert Haas wrote: (I note that the proposal to have the CFM review everything is merely one way of meeting the need to have senior people spend more time reviewing. But I assure all of you that I spend as much time reviewing as I can find time for. If someone wants to pay me the same salary I'm making now to do nothing but review patches, I'll think about it. But even then, that would also mean that I wasn't spending time writing patches of my own.) I have heard the idea of a cross-company PostgreSQL foundation of some sort that would hire a developer just to manage commitfests, do patch reviews, apply bugfixes, etc, without the obligations that come from individual companies' schedules for particular development roadmaps, customer support, and the like. Of course, only a senior person would be able to fill this role because it requires considerable experience. Yeah, that would be great, and even better if we could get 2 or 3 positions funded so that the success or failure isn't too much tied to a single individual. But even getting 1 position funded in a stable-enough fashion that someone would be willing to bet on it seems like a challenge. (Maybe other people here are less risk-averse than I am.) Yeah, it would be hard to sell anyone on that unless the foundation was so well funded that it could clearly afford to keep paying you for years into the future. I'm not really on board with the CFM-reviews-everything idea anyway. I don't think that can possibly work well, because it supposes that senior reviewers are interchangeable, which they aren't. Everybody's got pieces of the system that they know better than other pieces. Also, one part of the point of the review mechanism is that it's supposed to provide an opportunity for less-senior reviewers to look at parts of the code that they maybe don't know so well, and thereby help grow them into senior people. If we went over to the notion of some one (or a few) senior people doing all the reviewing, it might make the review process more expeditious but it would lose the training aspect. Of course, maybe the training aspect was never worth anything; I'm not in a position to opine on that. But I don't really think that centralizing that responsibility would be a good thing in the long run. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On Fri, Dec 12, 2014 at 10:50:56AM -0500, Tom Lane wrote: Also, one part of the point of the review mechanism is that it's supposed to provide an opportunity for less-senior reviewers to look at parts of the code that they maybe don't know so well, and thereby help grow them into senior people. If we went over to the notion of some one (or a few) senior people doing all the reviewing, it might make the review process more expeditious but it would lose the training aspect. Of course, maybe the training aspect was never worth anything; I'm not in a position to opine on that. But I don't really think that centralizing that responsibility would be a good thing in the long run. That is a very good point --- we have certainly had people doing reviews long enough to know if the review process is preparing developers for more complex tasks. I don't know the answer myself, which might say something. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On Fri, Dec 12, 2014 at 04:21:43PM +0100, Andres Freund wrote: On 2014-12-12 07:10:40 -0800, David Fetter wrote: On Thu, Dec 11, 2014 at 05:55:56PM -0500, Tom Lane wrote: Josh Berkus j...@agliodbs.com writes: How about *you* run the next one, Tom? I think the limited amount of time I can put into a commitfest is better spent on reviewing patches than on managing the process. With utmost respect, FWIW, the way you frequently use this phrase doesn't come over as actually being respectful. Respect is quantified, and in this case, the most afforded is the most earned. In the case of criticizing the work of others without an offer of help them do it better, respect for that behavior does have some pretty sharp upper limits, so yes, utmost is in that context. Tom, you seem to carve off an enormous amount of time to follow -bugs and -general. What say you unsubscribe to those lists for the duration of your tenure as CFM? And why on earth would that be a good idea? Because Tom Lane is not the person whose time is best spent screening these mailing lists. These bugs need to be fixed - we're actually behind on that front. So you're proposing a bug triage system, which is a separate discussion. Let's have that one in a separate thread. Are we now really trying to dictate how other developers manage their time? I was merely pointing out that time can be allocated, and that it appeared it could be allocated from a bucket for which persons less knowledgeable--perhaps a good bit less knowledgeable--about the entire code base than Tom are well suited. It's one thing to make up rules that say one review for one commit or something, And how do you think that would work out. Are you up for following it? it's something entirely else to try to assign tasks to them. I was, as I mentioned, merely pointing out that trade-offs are available. Cheers, David. -- David Fetter da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 12/12/2014 06:30 AM, Robert Haas wrote: Yeah, that would be great, and even better if we could get 2 or 3 positions funded so that the success or failure isn't too much tied to a single individual. But even getting 1 position funded in a stable-enough fashion that someone would be willing to bet on it seems like a challenge. (Maybe other people here are less risk-averse than I am.) Well, first, who would that person be? Last I checked, all of the senior committers were spoken for. I like this idea, but the list of people who could fill the role is pretty short, and I couldn't possibly start fundraising unless I had a candidate. Second, I don't think someone's employment will make a difference in fixing the commitfest and patch review *process* unless the contributors agree that it needs fixing and that they are willing to make changes to their individual workflow to fix it. Right now there is no consensus about moving forward in our patch review process; everyone seems to want the problem to go away without changing anything. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 12.12.2014 19:07, Bruce Momjian wrote: On Fri, Dec 12, 2014 at 10:50:56AM -0500, Tom Lane wrote: Also, one part of the point of the review mechanism is that it's supposed to provide an opportunity for less-senior reviewers to look at parts of the code that they maybe don't know so well, and thereby help grow them into senior people. If we went over to the notion of some one (or a few) senior people doing all the reviewing, it might make the review process more expeditious but it would lose the training aspect. Of course, maybe the training aspect was never worth anything; I'm not in a position to opine on that. But I don't really think that centralizing that responsibility would be a good thing in the long run. That is a very good point --- we have certainly had people doing reviews long enough to know if the review process is preparing developers for more complex tasks. I don't know the answer myself, which might say something. I can't speak for the others, but for me it certainly is a useful way to learn new stuff. Maybe not as important as working on my own patches, but it usually forces me to learn something new, and gives me a different perspective. regards Tomas -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 12 December 2014 at 15:10, David Fetter da...@fetter.org wrote: On Thu, Dec 11, 2014 at 05:55:56PM -0500, Tom Lane wrote: Josh Berkus j...@agliodbs.com writes: How about *you* run the next one, Tom? I think the limited amount of time I can put into a commitfest is better spent on reviewing patches than on managing the process. IIRC Tom was pretty much the only person doing patch review for probably 5 years during 2003-2008, maybe others. AFAICS he was managing that process. Thank you, Tom. I've never seen him moan loudly about this, so I'm surprised to hear such things from people that have done much less. Any solution to our current problems will come from working together, not by fighting. We just need to do more reviews. Realising this, I have begun to do more. I encourage others to do this also. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 12/11/2014 02:55 PM, Tom Lane wrote: Josh Berkus j...@agliodbs.com writes: How about *you* run the next one, Tom? I think the limited amount of time I can put into a commitfest is better spent on reviewing patches than on managing the process. Agreed but That means committers/hackers have to suck it up when the manager closes the commit fest. We don't get our cake and eat it too. We either accept that the CFM has the authority to do exactly what they are supposed to do, or we don't. JD -- Command Prompt, Inc. - http://www.commandprompt.com/ 503-667-4564 PostgreSQL Support, Training, Professional Services and Development High Availability, Oracle Conversion, @cmdpromptinc If we send our children to Caesar for their education, we should not be surprised when they come back as Romans. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 12/12/2014 06:30 AM, Robert Haas wrote: Yeah, that would be great, and even better if we could get 2 or 3 positions funded so that the success or failure isn't too much tied to a single individual. But even getting 1 position funded in a stable-enough fashion that someone would be willing to bet on it seems like a challenge. (Maybe other people here are less risk-averse than I am.) We (not CMD, the community) with proper incentive could fund this. It really wouldn't be that hard. That said, there would have to be a clear understanding of expectations, results, and authority. JD -- Command Prompt, Inc. - http://www.commandprompt.com/ 503-667-4564 PostgreSQL Support, Training, Professional Services and Development High Availability, Oracle Conversion, @cmdpromptinc If we send our children to Caesar for their education, we should not be surprised when they come back as Romans. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
On 12/12/2014 10:59 AM, Simon Riggs wrote: On 12 December 2014 at 15:10, David Fetter da...@fetter.org wrote: On Thu, Dec 11, 2014 at 05:55:56PM -0500, Tom Lane wrote: Josh Berkus j...@agliodbs.com writes: How about *you* run the next one, Tom? I think the limited amount of time I can put into a commitfest is better spent on reviewing patches than on managing the process. IIRC Tom was pretty much the only person doing patch review for probably 5 years during 2003-2008, maybe others. AFAICS he was managing that process. Thank you, Tom. I've never seen him moan loudly about this, so I'm surprised to hear such things from people that have done much less. Any solution to our current problems will come from working together, not by fighting. We just need to do more reviews. Realising this, I have begun to do more. I encourage others to do this also. Simon, Well said but again, I think a lot of people are hand waving about a simple problem (within the current structure) and that problem is just one of submission. Those doing the patch review/writing need to submit to the authority of the CFM or CFC (commit fest comittee). That happens and a lot of the angst around this process goes away. Sincerely, Joshua D. Drake -- Command Prompt, Inc. - http://www.commandprompt.com/ 503-667-4564 PostgreSQL Support, Training, Professional Services and Development High Availability, Oracle Conversion, @cmdpromptinc If we send our children to Caesar for their education, we should not be surprised when they come back as Romans. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Commitfest problems
Joshua D. Drake wrote: On 12/12/2014 06:30 AM, Robert Haas wrote: Yeah, that would be great, and even better if we could get 2 or 3 positions funded so that the success or failure isn't too much tied to a single individual. But even getting 1 position funded in a stable-enough fashion that someone would be willing to bet on it seems like a challenge. (Maybe other people here are less risk-averse than I am.) We (not CMD, the community) with proper incentive could fund this. It really wouldn't be that hard. That said, there would have to be a clear understanding of expectations, results, and authority. Uh, really? Last I looked at the numbers from SPI treasurer reports, they are not impressive enough to hire a full-time engineer, let alone a senior one. The Linux Foundation has managed to pay for Linus Torvalds somehow, so it does sound possible. We have a number of companies making money all over the globe, at least. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers