Обсуждение: Considering Gerrit for CFs
Hackers, As an occasional CommitFest manager, I'm keenly aware of the makeshift nature of the CommitFest app. If we want to go on using it -- and if we want to attract additional reviewers -- we need to improve it substantially. What Robert built for us was supposed to be a second draft, not a final version. The problem with doing it in-house is that the folks who can work on it and maintain it will be taking time away from developing PostgreSQL. So I've been keeping an eye on third-party OSS apps for contribution management, waiting for one of them to mature enough that we can seriously consider using it. I think one of them has, now: Gerrit. http://code.google.com/p/gerrit/ I spent some time with OpenStack's main Gerrit admin while at LCA, and was fairly encouraged that Gerrit would be a big step up compared to our current ad-hoc PHP. However, gerrit is designed to be git-centric rather than email-centric, so it would modify our current email-centric workflow (e.g. reviews are posted via a special git commit). Unlike other git tools, though, it expects patches and not branches, so that would integrate well with what we do now. It would also require supporting Java in our infrastructure. The advantages in features would be substantial: a better interface, ways to perform automated tasks (like remind submitters that a patch is waiting on author), online diffs, automated testing integration, and a configurable review workflow process. The existing Gerrit community would be keen to have the PostgreSQL project as a major user, though, and would theoretically help with modification needs. Current major users are OpenStack, Mediawiki, LibreOffice and QT. Thoughts? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
On Wed, Feb 6, 2013 at 10:07 PM, Josh Berkus <josh@agliodbs.com> wrote: > Hackers, > > As an occasional CommitFest manager, I'm keenly aware of the makeshift > nature of the CommitFest app. If we want to go on using it -- and if we > want to attract additional reviewers -- we need to improve it > substantially. What Robert built for us was supposed to be a second > draft, not a final version. This is probably not something we should discuss right now - it's better discussed when we're not right inthe middle of a commitfest, no? > The problem with doing it in-house is that the folks who can work on it > and maintain it will be taking time away from developing PostgreSQL. So > I've been keeping an eye on third-party OSS apps for contribution > management, waiting for one of them to mature enough that we can > seriously consider using it. > > I think one of them has, now: Gerrit. http://code.google.com/p/gerrit/ > > I spent some time with OpenStack's main Gerrit admin while at LCA, and > was fairly encouraged that Gerrit would be a big step up compared to our > current ad-hoc PHP. However, gerrit is designed to be git-centric We have no ad-hoc PHP, but I'm assume you're referring to the cf management app that's in perl? > rather than email-centric, so it would modify our current email-centric > workflow (e.g. reviews are posted via a special git commit). Unlike Previously, we've said we do not want to do this. And I think in general, it's a realliy bad idea to have a tool dictate the workflow. It should be the other way around. Now, if we *want' to change our workflow, that's a different story, of course. But a tool shouldn't dictate that. > other git tools, though, it expects patches and not branches, so that > would integrate well with what we do now. It would also require > supporting Java in our infrastructure. We already have a certain amount of support for java in the infrastructure. It does mandate that it doesn't have any special requirements on the java environment, of course - but as long as it works with the one that ships on Debian, we can do it. > The advantages in features would be substantial: a better interface, > ways to perform automated tasks (like remind submitters that a patch is > waiting on author), online diffs, automated testing integration, and a > configurable review workflow process. Could you point to an example somewhere that we could check such features out? > The existing Gerrit community would be keen to have the PostgreSQL > project as a major user, though, and would theoretically help with > modification needs. Current major users are OpenStack, Mediawiki, > LibreOffice and QT. "theoretically"? > Thoughts? I just took a quick look at their system, and when they start talking about requirements in the 100's of Gb of RAM, 24 core machines and SSD, I get scared :) But that's to "scale" it - doesn't mention when you need to do anything like that. I'm assuming we'd be tiny. FWIW, what we have now could easily run on a box with 128Mb RAM... --Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/
> This is probably not something we should discuss right now - it's > better discussed when we're not right inthe middle of a commitfest, > no? Well, *if* we were to change tooling, the time to do it would be during beta. Hence, bringing it up now. > We have no ad-hoc PHP, but I'm assume you're referring to the cf > management app that's in perl? Sorry, "ad-hoc perl". >> rather than email-centric, so it would modify our current email-centric >> workflow (e.g. reviews are posted via a special git commit). Unlike > > Previously, we've said we do not want to do this. And I think in > general, it's a realliy bad idea to have a tool dictate the workflow. > It should be the other way around. Yeah. It would be theoretically possible to change things so that Gerrit would accept email reviews, but anyone who's worked with RT can tell you that automated processing of email is error-prone. That's why nobody does it. Note that Gerrit is perfectly capable of *sending* email to the list, it's just *receiving* it which is an issue. Mind you, when I explained our current CF review workflow for the SF ReviewFest last year, the attendees thought I was insane. It's kept me from doing more reviewfests. Our current workflow and tooling is definitely a serious obstacle to gettng more reviewers. Seems like a good topic for the developer meeting. >> The advantages in features would be substantial: a better interface, >> ways to perform automated tasks (like remind submitters that a patch is >> waiting on author), online diffs, automated testing integration, and a >> configurable review workflow process. > > Could you point to an example somewhere that we could check such features out? This is a good place to start: https://review.openstack.org >> The existing Gerrit community would be keen to have the PostgreSQL >> project as a major user, though, and would theoretically help with >> modification needs. Current major users are OpenStack, Mediawiki, >> LibreOffice and QT. > > "theoretically"? Well, I think we learned from Gforge that folks are more willing to promise help than to deliver it. > I just took a quick look at their system, and when they start talking > about requirements in the 100's of Gb of RAM, 24 core machines and > SSD, I get scared :) But that's to "scale" it - doesn't mention when > you need to do anything like that. I'm assuming we'd be tiny. Yeah, I have no idea. Given that it's Java, I'd assume that requirements would go up. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
On 06/02/2013 22:25, Josh Berkus wrote: > Mind you, when I explained our current CF review workflow for the SF > ReviewFest last year, the attendees thought I was insane. It's kept me > from doing more reviewfests. Our current workflow and tooling is > definitely a serious obstacle to gettng more reviewers. Seems like a > good topic for the developer meeting. I'm honestly having a hard time believing this. I've never thought of the commitfest workflow as complicated or burdensome. I actually quite like how things are there at the moment. Besides, if we can't expect people to spend 30 minutes figuring out the workflow, how on earth do we expect them to review patches? Regards, Marko Tiikkaja
Magnus Hagander <magnus@hagander.net> writes:
> On Wed, Feb 6, 2013 at 10:07 PM, Josh Berkus <josh@agliodbs.com> wrote:
>> As an occasional CommitFest manager, I'm keenly aware of the makeshift
>> nature of the CommitFest app. If we want to go on using it -- and if we
>> want to attract additional reviewers -- we need to improve it
>> substantially. What Robert built for us was supposed to be a second
>> draft, not a final version.
> This is probably not something we should discuss right now - it's
> better discussed when we're not right inthe middle of a commitfest,
> no?
Judging from the last six months, it's doubtful whether we will ever
again not be in the middle of a commitfest :-(. If better tooling might
help us escape this state, we should talk about it. However ...
>> rather than email-centric, so it would modify our current email-centric
>> workflow (e.g. reviews are posted via a special git commit). Unlike
> Previously, we've said we do not want to do this. And I think in
> general, it's a realliy bad idea to have a tool dictate the workflow.
... if it's going to try to coerce us out of our email-centric habits,
then I for one am very much against it. To me, the problems with the
existing CF app are precisely that it's not well enough integrated with
the email discussions. The way to fix that is not to abandon email (or
at least, it's not a way I wish to pursue).
regards, tom lane
On 02/06/2013 01:53 PM, Tom Lane wrote: > ... if it's going to try to coerce us out of our email-centric habits, > then I for one am very much against it. To me, the problems with the > existing CF app are precisely that it's not well enough integrated with > the email discussions. The way to fix that is not to abandon email (or > at least, it's not a way I wish to pursue). The email centric habits are by far the biggest limiting factor we have. Email was never designed for integral collaboration. That said, I see no way around it. I have brought up this idea before but, Redmine has by far served the purposes (with a little effort) of CMD and it also integrates with GIT. It also does not break the email work flow. It does not currently allow commands via email but that could be easily (when I say easily, I mean it) added. Just another thought. JD -- Command Prompt, Inc. - http://www.commandprompt.com/ PostgreSQL Support, Training, Professional Services and Development High Availability, Oracle Conversion, Postgres-XC @cmdpromptinc - 509-416-6579
On Wed, Feb 06, 2013 at 10:17:09PM +0100, Magnus Hagander wrote: > I just took a quick look at their system, and when they start talking > about requirements in the 100's of Gb of RAM, 24 core machines and > SSD, I get scared :) But that's to "scale" it - doesn't mention when > you need to do anything like that. I'm assuming we'd be tiny. > > FWIW, what we have now could easily run on a box with 128Mb RAM... I'm right now setting up a gerrit instance for another project (not as large as Postgres) on a VM with 1GB of RAM and it works alright. You'll want to run a Postgres database next to it for storing the actual reviews and such. It's a nice tool and integrates reasonably well with other thing. We have a bot connected to it which sends messages on IRC in response to various state changes. I've never seen an instance respond to email though. Have a nice day, -- Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/ > He who writes carelessly confesses thereby at the very outset that he does > not attach much importance to his own thoughts. -- Arthur Schopenhauer
On Wed, Feb 6, 2013 at 3:00 PM, Joshua D. Drake <jd@commandprompt.com> wrote: > > On 02/06/2013 01:53 PM, Tom Lane wrote: > >> ... if it's going to try to coerce us out of our email-centric habits, >> then I for one am very much against it. To me, the problems with the >> existing CF app are precisely that it's not well enough integrated with >> the email discussions. The way to fix that is not to abandon email (or >> at least, it's not a way I wish to pursue). > > > The email centric habits are by far the biggest limiting factor we have. > Email was never designed for integral collaboration. That said, I see no way > around it. I have brought up this idea before but, Redmine has by far served > the purposes (with a little effort) of CMD and it also integrates with GIT. > It also does not break the email work flow. It does not currently allow > commands via email but that could be easily (when I say easily, I mean it) > added. > > Just another thought. I don't think controlling things by email is the issue. I have used the usual litany of bug trackers and appreciate the correspondence-driven model that -hackers and -bugs uses pretty pleasant. If nothing else, the uniform, well-developed, addressable, and indexed nature of the archives definitely provides me a lot of value that I haven't really seen duplicated in other projects that use structured bug or patch tracking. The individual communications tend to be of higher quality -- particularly to the purpose of later reference -- maybe a byproduct of the fact that prose is on the pedestal. There are obvious tooling gaps (aren't there always?), but I don't really see the model as broken, and I don't think I've been around pgsql-hackers exclusively or extensively enough to have developed Stockholm syndrome. I also happen to feel that the commitfest application works rather well for me in general. Sure, I wish that it might slurp up all submitted patches automatically or something like that with default titles or something or identify new versions when they appear, but I've always felt that skimming the commitfest detail page for a patch was useful. It was perhaps harder to know if the commitfest page I was looking at was complete or up to date or not, although it often is. Here's what I find most gut-wrenching in the whole submit-review-commit process: * When a reviewer shows up a couple of weeks later and says "this patch doesn't apply" or "make check crash" or "fails to compile". It's especially sad because the reviewer has presumably cut out a chunk of time -- now probably aborted -- to review the patch. Machines can know these things automatically. * When on occasion patches are submitted with non-C/sh/perl suites that may not really be something that anyone wants to be a build/source tree, but seem like they might do a better job testing the patch. The inevitable conclusion is that the automated test harness is tossed, or never written because it is known it will have no place to live after a possible commit. Somehow I wish those could live somewhere and run against all submitted patches. I've toyed a very, very tiny bit with running build farm clients on Heroku with both of these in mind, but haven't revisited it in a while. -- fdr
On 7 February 2013 08:07, Josh Berkus <josh@agliodbs.com> wrote: > The existing Gerrit community would be keen to have the PostgreSQL > project as a major user, though, and would theoretically help with > modification needs. Current major users are OpenStack, Mediawiki, > LibreOffice and QT. Do we actually have any requirements that are known to be uncatered for in gerrit's standard feature set? Cheers, BJ
On 2013-02-06 13:25:31 -0800, Josh Berkus wrote: > Mind you, when I explained our current CF review workflow for the SF > ReviewFest last year, the attendees thought I was insane. It's kept me > from doing more reviewfests. Our current workflow and tooling is > definitely a serious obstacle to gettng more reviewers. Seems like a > good topic for the developer meeting. I personally feel that the CF process isn't limited by technicalities like the commitfest UI or by the lack of random reviewers. Its limited by the amount of available reviewer time of people with in-depth knowledge of pg and committer bandwith. Trying to solve that problem via technical means doesn't seem likely to work out very well. > >> The existing Gerrit community would be keen to have the PostgreSQL > >> project as a major user, though, and would theoretically help with > >> modification needs. Current major users are OpenStack, Mediawiki, > >> LibreOffice and QT. There's also android as a rather major user... > > I just took a quick look at their system, and when they start talking > > about requirements in the 100's of Gb of RAM, 24 core machines and > > SSD, I get scared :) But that's to "scale" it - doesn't mention when > > you need to do anything like that. I'm assuming we'd be tiny. Well, with gerrit you do far more work inside the application in comparison to what we are doing today in the CF app, so part of this probably is just that more work is running through it... Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
2013/2/7 Andres Freund <andres@2ndquadrant.com>: > On 2013-02-06 13:25:31 -0800, Josh Berkus wrote: >> Mind you, when I explained our current CF review workflow for the SF >> ReviewFest last year, the attendees thought I was insane. It's kept me >> from doing more reviewfests. Our current workflow and tooling is >> definitely a serious obstacle to gettng more reviewers. Seems like a >> good topic for the developer meeting. > > I personally feel that the CF process isn't limited by technicalities > like the commitfest UI or by the lack of random reviewers. Its limited > by the amount of available reviewer time of people with in-depth > knowledge of pg and committer bandwith. > Trying to solve that problem via technical means doesn't seem likely to > work out very well. +1 Pavel > >> >> The existing Gerrit community would be keen to have the PostgreSQL >> >> project as a major user, though, and would theoretically help with >> >> modification needs. Current major users are OpenStack, Mediawiki, >> >> LibreOffice and QT. > > There's also android as a rather major user... > >> > I just took a quick look at their system, and when they start talking >> > about requirements in the 100's of Gb of RAM, 24 core machines and >> > SSD, I get scared :) But that's to "scale" it - doesn't mention when >> > you need to do anything like that. I'm assuming we'd be tiny. > > Well, with gerrit you do far more work inside the application in > comparison to what we are doing today in the CF app, so part of this > probably is just that more work is running through it... > > 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
On Thu, Feb 7, 2013 at 8:20 AM, Daniel Farina <daniel@heroku.com> wrote: > On Wed, Feb 6, 2013 at 3:00 PM, Joshua D. Drake <jd@commandprompt.com> wrote: >> >> On 02/06/2013 01:53 PM, Tom Lane wrote: >> >>> ... if it's going to try to coerce us out of our email-centric habits, >>> then I for one am very much against it. To me, the problems with the >>> existing CF app are precisely that it's not well enough integrated with >>> the email discussions. The way to fix that is not to abandon email (or >>> at least, it's not a way I wish to pursue). >> >> >> The email centric habits are by far the biggest limiting factor we have. >> Email was never designed for integral collaboration. That said, I see no way >> around it. I have brought up this idea before but, Redmine has by far served >> the purposes (with a little effort) of CMD and it also integrates with GIT. >> It also does not break the email work flow. It does not currently allow >> commands via email but that could be easily (when I say easily, I mean it) >> added. >> >> Just another thought. > > I don't think controlling things by email is the issue. I have used > the usual litany of bug trackers and appreciate the > correspondence-driven model that -hackers and -bugs uses pretty > pleasant. If nothing else, the uniform, well-developed, addressable, > and indexed nature of the archives definitely provides me a lot of > value that I haven't really seen duplicated in other projects that use > structured bug or patch tracking. The individual communications tend > to be of higher quality -- particularly to the purpose of later > reference -- maybe a byproduct of the fact that prose is on the > pedestal. > > There are obvious tooling gaps (aren't there always?), but I don't > really see the model as broken, and I don't think I've been around > pgsql-hackers exclusively or extensively enough to have developed > Stockholm syndrome. > > I also happen to feel that the commitfest application works rather > well for me in general. Sure, I wish that it might slurp up all > submitted patches automatically or something like that with default > titles or something or identify new versions when they appear, but > I've always felt that skimming the commitfest detail page for a patch > was useful. It was perhaps harder to know if the commitfest page I > was looking at was complete or up to date or not, although it often > is. > > Here's what I find most gut-wrenching in the whole submit-review-commit process: > > * When a reviewer shows up a couple of weeks later and says "this > patch doesn't apply" or "make check crash" or "fails to compile". > It's especially sad because the reviewer has presumably cut out a > chunk of time -- now probably aborted -- to review the patch. > Machines can know these things automatically. If the report is *just* "this patch doesn't apply", I agree. If the reviewer is more "this patch doesn't apply anymore. Here's an adjusted version that does" it has a value in itself - because somebody else just got more familiar with that part of the code. > * When on occasion patches are submitted with non-C/sh/perl suites > that may not really be something that anyone wants to be a > build/source tree, but seem like they might do a better job testing > the patch. The inevitable conclusion is that the automated test > harness is tossed, or never written because it is known it will have > no place to live after a possible commit. Somehow I wish those could > live somewhere and run against all submitted patches. > > I've toyed a very, very tiny bit with running build farm clients on > Heroku with both of these in mind, but haven't revisited it in a > while. It's certainly been loosely discusse dbefore as a possible enhancement - but the main thing being it basically *has* to be run in a virtualized environment that's thrown away, or you're going to open all sorts of issues with running arbitrary code on peoples machines. Of course, virtualization is kind of what you guys do :) Personally, I find the most annoying thing with the current process being when reviewers post their reviews as a completely separate thread, instead of replying in the original thread. This causes context switches when parsing things, because now you have to jump back and forth between the CF app and your mail reader. But it's still only on the annoyance side, I think the process in general is not broken. (That said, I *have* been on the inside a long time, *and* I live in Stockholm, so I might well have that syndrome) --Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/
On Thu, Feb 7, 2013 at 8:29 AM, Brendan Jurd <direvus@gmail.com> wrote: > On 7 February 2013 08:07, Josh Berkus <josh@agliodbs.com> wrote: >> The existing Gerrit community would be keen to have the PostgreSQL >> project as a major user, though, and would theoretically help with >> modification needs. Current major users are OpenStack, Mediawiki, >> LibreOffice and QT. > > Do we actually have any requirements that are known to be uncatered > for in gerrit's standard feature set? Email being the primary interaction method has long been a stated requirement, and we've just been told that's uncatered for in gerrit. Turning the same question around, do we have any requirements on top of the current CF app that actually *are* catered for in gerrit? That is, what problem are we actually trying to solve? (On a technical level - "bring in more reviewers" doesn't count, you have to explain *how* that's going to happen) --Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/
Folks, First, thanks for the serious discussion of this. >> There are obvious tooling gaps (aren't there always?), but I don't >> really see the model as broken, and I don't think I've been around >> pgsql-hackers exclusively or extensively enough to have developed >> Stockholm syndrome. I don't see the model as broken either. Just the tooling, which is why I'm looking at tooling. As in, I'm looking for better tooling in order to solve two problems: 1. maximize the efficiency of existing reviewer time 2. make tooling not be an obstacle to getting new reviewers Of these two, (2) is actually the more critical. We have been losing, not gaining, active committers and reviewers for the last couple years.Clearly "do more of what we've been doing" is a losingstrategy. We need to be sucessfully moving people up the contributor chain if we're ever going to get out of this "not enough reviewers" hole. I agree that tooling is a minority of this, but tooling is also the easiest thing to change (compared with project organization), so that's what I'm tackling first. Expect a discussion on the people aspects at the developer meeting. > Personally, I find the most annoying thing with the current process > being when reviewers post their reviews as a completely separate > thread, instead of replying in the original thread. This causes > context switches when parsing things, because now you have to jump > back and forth between the CF app and your mail reader. But it's still > only on the annoyance side, I think the process in general is not > broken. (That said, I *have* been on the inside a long time, *and* I > live in Stockholm, so I might well have that syndrome) So, look at this from the perspective of a casual reviewer, say at a PUG reviewfest. Instructions to new reviewer: 1. find the feature you want to review on the CF app. 2. Click the link to the mailing list archives. 3. Click all the way through the list thread to make sure there isn't a later version of the patch. 4. Download the patch. Hopefully it's not mangled by the archives (this has gotten much better than it was last year) 5. Apply the patch to HEAD clone. 6. Do actual reviewing/testing. 7. Write an email review. 8. Send it to pgsql-hackers 8.a. this requires you to be subscribed to pgsql-hackers. 9. wait for the email to show up in the archives. 10. create a review comment in the CF app. 10.a. this requires you to be signed up for a community account 10.b. sign up for one now 10.c. wait for it to be approved 11. link the review comment to the messageID 12. change status of the patch This is a few too many steps, and certainly appears completely broken to any newcomer. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
On Fri, Feb 8, 2013 at 1:32 AM, Josh Berkus <josh@agliodbs.com> wrote: > Folks, > > First, thanks for the serious discussion of this. > >>> There are obvious tooling gaps (aren't there always?), but I don't >>> really see the model as broken, and I don't think I've been around >>> pgsql-hackers exclusively or extensively enough to have developed >>> Stockholm syndrome. > > I don't see the model as broken either. Just the tooling, which is why > I'm looking at tooling. As in, I'm looking for better tooling in order Yet you are suggesting tooling that requires a change in the model? > to solve two problems: > > 1. maximize the efficiency of existing reviewer time > > 2. make tooling not be an obstacle to getting new reviewers I think you are missing a fundamental part in this - which is "0. don't negatively affect the efficiency of existing committer time". I'm not saying it necessarily does (though I think it does, but that's not a proven point), but that has to be a pretty high priority. > Of these two, (2) is actually the more critical. We have been losing, > not gaining, active committers and reviewers for the last couple years. > Clearly "do more of what we've been doing" is a losing strategy. We > need to be sucessfully moving people up the contributor chain if we're > ever going to get out of this "not enough reviewers" hole. Agreed. But do you have any actual proof that the problem is in "we loose reviewers because we're relying on email"? > I agree that tooling is a minority of this, but tooling is also the > easiest thing to change (compared with project organization), so that's > what I'm tackling first. Expect a discussion on the people aspects at > the developer meeting. It would probably be a good thing to discuss the tooling there, too. >> Personally, I find the most annoying thing with the current process >> being when reviewers post their reviews as a completely separate >> thread, instead of replying in the original thread. This causes >> context switches when parsing things, because now you have to jump >> back and forth between the CF app and your mail reader. But it's still >> only on the annoyance side, I think the process in general is not >> broken. (That said, I *have* been on the inside a long time, *and* I >> live in Stockholm, so I might well have that syndrome) > > So, look at this from the perspective of a casual reviewer, say at a PUG > reviewfest. Instructions to new reviewer: > > 1. find the feature you want to review on the CF app. > > 2. Click the link to the mailing list archives. > > 3. Click all the way through the list thread to make sure there isn't a > later version of the patch. Supposedly the latest version should always be listed in the CF app. The fact that this is a manual step is a problem, that could probably be fixed quite easily. > 4. Download the patch. Hopefully it's not mangled by the archives > (this has gotten much better than it was last year) Yes, several things have been done to make this work better. There shouldn't be any issues at all with it now - and if there are, we are in a much better position to fix them. > 5. Apply the patch to HEAD clone. > > 6. Do actual reviewing/testing. > > 7. Write an email review. > > 8. Send it to pgsql-hackers > 8.a. this requires you to be subscribed to pgsql-hackers. No, it does not. It will get caught in the moderation queue and get slightly delayed if you're not, but it works perfectly fine. And if we were to use another system, you'd still have to sign up for that one, so it's not really that big a problem. > 9. wait for the email to show up in the archives. You do realize this currently counts within seconds or something like that, rather than the 15+ minutes it used to be? And the fact is, you don't actually have to wait for it. > 10. create a review comment in the CF app. > 10.a. this requires you to be signed up for a community account > 10.b. sign up for one now > 10.c. wait for it to be approved Huh? There is no approval process for community accounts. There is a verification step, of course, but any system would have that. > 11. link the review comment to the messageID > > 12. change status of the patch > > This is a few too many steps, and certainly appears completely broken to > any newcomer. I agree it's way too many step. Several of those can certainly be made more efficient now that we have a more sane archives, well within the scope of the current system. --Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/
On 2/6/13 4:07 PM, Josh Berkus wrote: > I think one of them has, now: Gerrit. http://code.google.com/p/gerrit/ I find Gerrit pretty useful, and I would support trying it out. I suggest, build it and they will come, or not. Let people push their patches into Gerrit and attach the reviews to the commit fest items. If reviewers then want to use that, it's their choice. We'll see how it goes. You don't need to replace the commitfest app (if that's what you were getting at), and Gerrit isn't an issue tracking system anyway.
On 2/8/13 5:23 AM, Magnus Hagander wrote: > But do you have any actual proof that the problem is in "we > loose reviewers because we're relying on email"? Here is one: Me. Just yesterday I downloaded a piece of software that was previously unknown to me from GitHub and found a bug. Within 15 minutes or so I had fixed the bug, made a fork, sent a pull request. Today I read, the fix was merged last night, and I'm happy. How would this go with PostgreSQL? You can use the bug form on the web site, but you can't attach any code, so the bug will just linger and ultimately put more burden on a core contributor to deal with the minutiae of developing, testing, and committing a trivial fix and sending feedback to the submitter. Or the user could take the high road and develop and patch and submit it. Just make sure it's in context diff format! Search the wiki if you don't know how to do that! Send it to -hackers, your email will be held for moderation. We won't actually do anything with your patch, but we will tell you to add it to that commitfest app over there. You need to sign up for an account to use that. We will deal with your patch in one or two months. But only if you review another patch. And you should sign up for that other mailing list, to make sure you're doing it right. Chances are, the first review you're going to get is that your patch doesn't apply anymore, but which time you will have lost interest in the patch anyway. So, I don't have any further evidence that we are losing reviewers, but in light of the above and the options out there were interested developers can contribute much more easily, I'm amazed that we are getting any new contributors or reviewers at all. Of course, Gerrit doesn't actually address most of the issues above, but it could be part of a step forward.
On Fri, Feb 8, 2013 at 4:20 PM, Peter Eisentraut <peter_e@gmx.net> wrote: > On 2/8/13 5:23 AM, Magnus Hagander wrote: >> But do you have any actual proof that the problem is in "we >> loose reviewers because we're relying on email"? > > Here is one: Me. > > Just yesterday I downloaded a piece of software that was previously > unknown to me from GitHub and found a bug. Within 15 minutes or so I > had fixed the bug, made a fork, sent a pull request. Today I read, the > fix was merged last night, and I'm happy. > > How would this go with PostgreSQL? You can use the bug form on the web > site, but you can't attach any code, so the bug will just linger and > ultimately put more burden on a core contributor to deal with the > minutiae of developing, testing, and committing a trivial fix and > sending feedback to the submitter. Or the user could take the high road > and develop and patch and submit it. Just make sure it's in context > diff format! Search the wiki if you don't know how to do that! Send it > to -hackers, your email will be held for moderation. We won't actually > do anything with your patch, but we will tell you to add it to that > commitfest app over there. You need to sign up for an account to use > that. We will deal with your patch in one or two months. But only if > you review another patch. And you should sign up for that other mailing > list, to make sure you're doing it right. Chances are, the first review > you're going to get is that your patch doesn't apply anymore, but which > time you will have lost interest in the patch anyway. > > So, I don't have any further evidence that we are losing reviewers, but > in light of the above and the options out there were interested > developers can contribute much more easily, I'm amazed that we are > getting any new contributors or reviewers at all. > > Of course, Gerrit doesn't actually address most of the issues above, but > it could be part of a step forward. You're outlining an issue for submitters. Berkus was complaining about issues for reviewers. These are clearly different issues. And I don't think gerrit helps at all with the submitters process that you've outlined above - it's a tool to help the reviewing. That doesn't, of course, mean that we shouldn't try to solve both things - but they are completely different. Basically, what you're saying above, is we should start accepting pull requests from github. There's nothing preventing us from doing that (other than the wish to do so), no need to change tooling for review for that. It just means that committers need to use git and add peoples repositories as remotes instead of applying patches. Probably not a huge burden today since most pg developers are used to git by now. However, it's not going to change the requirement to help review other things, that's a pure policy issue. Which I'm pretty sure we don't enforce for 10-minute-trivial-fixup-patches. And it's not going to change the fact that it takes time before someone gets around to your patch, that's a resource issue. And it's not going to change the fact that a patch migt not apply after 2 months, that's a consequence of the second problem. It doesn't change the fact that you have to sign up - it just makes it more likely that you're already signed up, since so many people are on github already, but you *do* have to sign up for a service, wherever it's hosted. But it does change the fact that you don't have to deal with email, and can use web instead. Personally, I find it much easier to just "git clone, make changes, git diff, attach to email", than "fork on github. git clone, make changes, push to github, create pull request on github. repeatedly check status of said pull request since email notifications aren't usable". But that's me, personally, and I realize many people today prefer web interfaces for as much as possible. There's nothing stopping us from supporting both, of course. --Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/
Peter Eisentraut <peter_e@gmx.net> writes:
> I suggest, build it and they will come, or not. Let people push their
> patches into Gerrit and attach the reviews to the commit fest items. If
> reviewers then want to use that, it's their choice. We'll see how it goes.
I might be misunderstanding what you're suggesting here, but it sounds
like this would imply that reviews could end up off in a Gerrit repo
somewhere, never getting posted to the mailing lists at all. That would
make me sad. The list archives are this project's community memory,
and I have every expectation that they'll still be around and useful
when Gerrit is forgotten. I don't object to people using their
tools-of-choice to perform reviewing, but we need some way of making
sure that the reviews get archived.
regards, tom lane
-----BEGIN PGP SIGNED MESSAGE----- Hash: RIPEMD160 > The problem with doing it in-house is that the folks who can work on it > and maintain it will be taking time away from developing PostgreSQL. Not sure that using Gerrit solves this. Someone will need to install it, maintain it, document, and hack it. Yes, hack it, as it is not a drop-in solution. ... > I think one of them has, now: Gerrit. http://code.google.com/p/gerrit/ I use Gerrit in the MediaWiki project, and it ain't pretty. The interface is confusing, the workflow is more complex, and the MediaWiki folks have had to do a lot of work to make things usable, despite their having a non-email-centric workflow already. Maybe we can identify specific issues with our current app instead? - -- Greg Sabino Mullane greg@turnstep.com PGP Key: 0x14964AC8 201302081106 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -----BEGIN PGP SIGNATURE----- iEYEAREDAAYFAlEVIqcACgkQvJuQZxSWSsh04gCfaK80dbuL8NnAVuViGR5sFQXN GzwAoM+2fcI6+zFZPqkslZrWjkZ05AOo =azLj -----END PGP SIGNATURE-----
-----BEGIN PGP SIGNED MESSAGE----- Hash: RIPEMD160 > How would this go with PostgreSQL? You can use the bug form on the web > site, but you can't attach any code, so the bug will just linger and > ultimately put more burden on a core contributor to deal with the > minutiae of developing, testing, and committing a trivial fix and > sending feedback to the submitter. Well, they could attach a link to a github patch... > Or the user could take the high road and develop and patch and submit it. > Just make sure it's in context diff format! Search the wiki if you don't > know how to do that! Send it to -hackers, your email will be held for > moderation. We won't actually do anything with your patch, but we will > tell you to add it to that commitfest app over there. You need to sign up > for an account to use that. We will deal with your patch in one or two months. > But only if you review another patch. And you should sign up for that > other mailing list, to make sure you're doing it right. Chances are, the > first review you're going to get is that your patch doesn't apply anymore, > but which time you will have lost interest in the patch anyway. +1 to all that. Especially the signing up for the commitfest app. > Of course, Gerrit doesn't actually address most of the issues above, but > it could be part of a step forward. More of a step sideways. It doesn't address the bigger problems. - -- Greg Sabino Mullane greg@turnstep.com PGP Key: 0x14964AC8 201302081124 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -----BEGIN PGP SIGNATURE----- iEYEAREDAAYFAlEVJrkACgkQvJuQZxSWSshh3gCgz+XHwAbk5rryttYPi68j4EJi 7DcAnjEdxDD4Rm2/oDBaqHbOzQLwR6zR =0lnp -----END PGP SIGNATURE----- 2~
On 02/08/2013 07:58 AM, Tom Lane wrote: > Peter Eisentraut <peter_e@gmx.net> writes: >> I suggest, build it and they will come, or not. Let people push their >> patches into Gerrit and attach the reviews to the commit fest items. If >> reviewers then want to use that, it's their choice. We'll see how it goes. > > I might be misunderstanding what you're suggesting here, but it sounds > like this would imply that reviews could end up off in a Gerrit repo > somewhere, never getting posted to the mailing lists at all. That would > make me sad. It would also be ineffective. I already tried having a secondary review forum for people who wanted to do reviews but not subscribe to -hackers; it didn't work out too well. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
>> I don't see the model as broken either. Just the tooling, which is why >> I'm looking at tooling. As in, I'm looking for better tooling in order > > Yet you are suggesting tooling that requires a change in the model? Well, my fantasy is a version of Gerrit which accepts email from -hackers and proceeds accordingly. However, I also happen to know intimately how difficult automated processing of email is -- if we wait for a tool which can do this, we're going to be stuck with the existing system forever. > I think you are missing a fundamental part in this - which is "0. > don't negatively affect the efficiency of existing committer time". > I'm not saying it necessarily does (though I think it does, but that's > not a proven point), but that has to be a pretty high prity. Realistically, the only way out of the current committer bottleneck is to recruit more reviewers, contributors, and committers. In the 9.3 timeline, we're going to have to look at ways we believe will accomplish recruitment and promotion, even if it means sacrificing committer time (and thus, 9.4 features) in the short run. If we remain focused on maximizing the time of existing major contributors to the exclusion of recruitment, things will never get better. Or to put it another way: as the EU has proven, Austerity Plans are a loser's game. >> Of these two, (2) is actually the more critical. We have been losing, >> not gaining, active committers and reviewers for the last couple years. >> Clearly "do more of what we've been doing" is a losing strategy. We >> need to be sucessfully moving people up the contributor chain if we're >> ever going to get out of this "not enough reviewers" hole. > > Agreed. But do you have any actual proof that the problem is in "we > loose reviewers because we're relying on email"? I don't think email is the specific issue. I think the issues are mostly people issues. The only reason I care about email vs. not-email is the technical impossibility of developing a system which can automatically turn patch and review emails into a trackable and transparent view. There are a bunch of other issues I'd like to discuss, but I agree that they should wait until after CF4. > It would probably be a good thing to discuss the tooling there, too. Yes. > I agree it's way too many step. Several of those can certainly be made > more efficient now that we have a more sane archives, well within the > scope of the current system. Right. My concern is that the people who have to do that are exactly the people whose time is already the most scarce. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
On 2/8/13 10:58 AM, Tom Lane wrote: > Peter Eisentraut <peter_e@gmx.net> writes: >> I suggest, build it and they will come, or not. Let people push their >> patches into Gerrit and attach the reviews to the commit fest items. If >> reviewers then want to use that, it's their choice. We'll see how it goes. > > I might be misunderstanding what you're suggesting here, but it sounds > like this would imply that reviews could end up off in a Gerrit repo > somewhere, never getting posted to the mailing lists at all. That would > make me sad. The list archives are this project's community memory, > and I have every expectation that they'll still be around and useful > when Gerrit is forgotten. I don't object to people using their > tools-of-choice to perform reviewing, but we need some way of making > sure that the reviews get archived. Gerrit sends me an email every times something happens, so I think this is not going to be a problem. What it doesn't support AFAICT is sending emails *in*, but I don't see that as a requirement.
Peter Eisentraut <peter_e@gmx.net> writes:
> On 2/8/13 10:58 AM, Tom Lane wrote:
>> ... I don't object to people using their
>> tools-of-choice to perform reviewing, but we need some way of making
>> sure that the reviews get archived.
> Gerrit sends me an email every times something happens, so I think this
> is not going to be a problem.
As long as it can be persuaded to mail the text of reviews (not just a
notification), that'd probably be all right.
> What it doesn't support AFAICT is sending emails *in*, but I don't see
> that as a requirement.
Meh. Many of the complaints about the current CF application boil down
to the fact that it doesn't accept email input, so I'm not really
convinced that a different tool that also doesn't accept email input
is going to be a big step forward.
regards, tom lane
I thought this might be of interest... http://blog.documentfoundation.org/2013/02/07/the-document-foundation-announces-libreoffice-4-0/ [...] Improved code contribution thanks to Gerrit: a web based code review system, facilitating the task for projects using Git version control system (although this is not specific of LibreOffice 4.0, it has entered the production stage just before the 4.0 branch) [...]. Cheers, Gavin
On Fri, Feb 8, 2013 at 10:20 AM, Peter Eisentraut <peter_e@gmx.net> wrote: > On 2/8/13 5:23 AM, Magnus Hagander wrote: >> But do you have any actual proof that the problem is in "we >> loose reviewers because we're relying on email"? > > Here is one: Me. > > Just yesterday I downloaded a piece of software that was previously > unknown to me from GitHub and found a bug. Within 15 minutes or so I > had fixed the bug, made a fork, sent a pull request. Today I read, the > fix was merged last night, and I'm happy. > > How would this go with PostgreSQL? You can use the bug form on the web > site, but you can't attach any code, so the bug will just linger and > ultimately put more burden on a core contributor to deal with the > minutiae of developing, testing, and committing a trivial fix and > sending feedback to the submitter. Or the user could take the high road > and develop and patch and submit it. Just make sure it's in context > diff format! Search the wiki if you don't know how to do that! Send it > to -hackers, your email will be held for moderation. We won't actually > do anything with your patch, but we will tell you to add it to that > commitfest app over there. You need to sign up for an account to use > that. We will deal with your patch in one or two months. But only if > you review another patch. And you should sign up for that other mailing > list, to make sure you're doing it right. Chances are, the first review > you're going to get is that your patch doesn't apply anymore, but which > time you will have lost interest in the patch anyway. This. This times 1000. > > So, I don't have any further evidence that we are losing reviewers, but > in light of the above and the options out there were interested > developers can contribute much more easily, I'm amazed that we are > getting any new contributors or reviewers at all. > > Of course, Gerrit doesn't actually address most of the issues above, but > it could be part of a step forward. > I'm not sure if Gerrit specifically is the answer, but there are definitely better ways to do code review like this. I really like the way github allows you to post a patch and then have conversation around it, offer comments on specific lines of code, and add updates to the patch all in one interface. Another benefit is that a lot more people are familiar and comfortable with this work flow. There are even some open source work-a-likes that we could use to we don't have to rely on a 3rd party like github. Gerrit seems to do it slightly differently with side by side diff's and patch revisions, but either way would be an improvement. I understand there are other concerns in this thread, like email, etc. I don't have a comprehensive plan that solves all this, but I wanted to add my +1 to the idea of something more sophisticated when it comes to code review. > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Feb 8, 2013 at 7:20 AM, Peter Eisentraut <peter_e@gmx.net> wrote: > On 2/8/13 5:23 AM, Magnus Hagander wrote: >> But do you have any actual proof that the problem is in "we >> loose reviewers because we're relying on email"? > > Here is one: Me. > > Just yesterday I downloaded a piece of software that was previously > unknown to me from GitHub and found a bug. Within 15 minutes or so I > had fixed the bug, made a fork, sent a pull request. Today I read, the > fix was merged last night, and I'm happy. I know quite a bit using git for my own work, but I haven't the foggiest idea how to make a fork (unless that is the same as making a branch?) or to send a pull request, and bet it would take me more than 15 minutes to figure it out and make sure I understood them and did it correctly. Surely using any specific tool would make things easier for that pool of people who are already well versed in that tool. > How would this go with PostgreSQL? You can use the bug form on the web > site, but you can't attach any code, Should it allow you to attach code? If you have code to attach, should it instead go to hackers? Or send it to bugs by email rather than using the form? (Some parts of the web site sound like the form is preferred over direct email to bugs, while others make it sound that both are equal) > so the bug will just linger and > ultimately put more burden on a core contributor to deal with the > minutiae of developing, testing, and committing a trivial fix and > sending feedback to the submitter. Or the user could take the high road > and develop and patch and submit it. Just make sure it's in context > diff format! Search the wiki if you don't know how to do that! Send it > to -hackers, your email will be held for moderation. We won't actually > do anything with your patch, That sounds more like a feature submission. My experience with bugs is that I send a patch or just a code snippet, either to hackers or bugs, and then someone, usually Tom, rewrites it to be better and to work for corner-cases, then commits it. Only for complicated bugs that are arguably not really bugs but rather mal-features that need to be redesigned would I be asked to use the commitfest process at all. > but we will tell you to add it to that > commitfest app over there. You need to sign up for an account to use > that. We will deal with your patch in one or two months. But only if > you review another patch. And you should sign up for that other mailing > list, to make sure you're doing it right. Chances are, the first review > you're going to get is that your patch doesn't apply anymore, but which > time you will have lost interest in the patch anyway. This too does not sound like how bug reports actually work. Nor does it sound like how very simple enhancements work (i.e. several of my tab-completion enhancements or doc changes), which are usually just summarily committed regardless of the commitfest cycle or whether I review other patches. You are comparing making a drive-by contribution to one project, to being part of the developer community of a different one. Cheers, Jeff
On Fri, Feb 8, 2013 at 2:23 AM, Magnus Hagander <magnus@hagander.net> wrote: > On Fri, Feb 8, 2013 at 1:32 AM, Josh Berkus <josh@agliodbs.com> wrote: >> >> 8. Send it to pgsql-hackers >> 8.a. this requires you to be subscribed to pgsql-hackers. > > No, it does not. It will get caught in the moderation queue and get > slightly delayed if you're not, but it works perfectly fine. http://www.postgresql.org/docs/current/static/bug-reporting.html "Note: Due to the unfortunate amount of spam going around, all of the above email addresses are closed mailing lists. That is, you need to be subscribed to a list to be allowed to post on it." Is this wrong, or just strategically over-simplified? Cheers, Jeff
Jeff Janes escribió: > On Fri, Feb 8, 2013 at 2:23 AM, Magnus Hagander <magnus@hagander.net> wrote: > > On Fri, Feb 8, 2013 at 1:32 AM, Josh Berkus <josh@agliodbs.com> wrote: > >> > >> 8. Send it to pgsql-hackers > >> 8.a. this requires you to be subscribed to pgsql-hackers. > > > > No, it does not. It will get caught in the moderation queue and get > > slightly delayed if you're not, but it works perfectly fine. > > http://www.postgresql.org/docs/current/static/bug-reporting.html > > "Note: Due to the unfortunate amount of spam going around, all of the > above email addresses are closed mailing lists. That is, you need to > be subscribed to a list to be allowed to post on it." > > Is this wrong, or just strategically over-simplified? Well, it is factually wrong, because the moderators will see such requests and approve them. And the parenthical remark following the quoted part is wrong too, in a sense, because even though using the form does not require you to be subscribed, the report will not be posted until a moderator approves it; so it has the same problem. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On Fri, Feb 8, 2013 at 1:43 PM, Phil Sorber <phil@omniti.com> wrote: > On Fri, Feb 8, 2013 at 10:20 AM, Peter Eisentraut <peter_e@gmx.net> wrote: >> On 2/8/13 5:23 AM, Magnus Hagander wrote: >>> But do you have any actual proof that the problem is in "we >>> loose reviewers because we're relying on email"? >> >> Here is one: Me. >> >> Just yesterday I downloaded a piece of software that was previously >> unknown to me from GitHub and found a bug. Within 15 minutes or so I >> had fixed the bug, made a fork, sent a pull request. Today I read, the >> fix was merged last night, and I'm happy. >> >> How would this go with PostgreSQL? You can use the bug form on the web >> site, but you can't attach any code, so the bug will just linger and >> ultimately put more burden on a core contributor to deal with the >> minutiae of developing, testing, and committing a trivial fix and >> sending feedback to the submitter. Or the user could take the high road >> and develop and patch and submit it. Just make sure it's in context >> diff format! Search the wiki if you don't know how to do that! Send it >> to -hackers, your email will be held for moderation. We won't actually >> do anything with your patch, but we will tell you to add it to that >> commitfest app over there. You need to sign up for an account to use >> that. We will deal with your patch in one or two months. But only if >> you review another patch. And you should sign up for that other mailing >> list, to make sure you're doing it right. Chances are, the first review >> you're going to get is that your patch doesn't apply anymore, but which >> time you will have lost interest in the patch anyway. > > This. This times 1000. I, too, could not agree more. > I'm not sure if Gerrit specifically is the answer, but there are > definitely better ways to do code review like this. I really like the > way github allows you to post a patch and then have conversation > around it, offer comments on specific lines of code, and add updates > to the patch all in one interface. Another benefit is that a lot more > people are familiar and comfortable with this work flow. There are > even some open source work-a-likes that we could use to we don't have > to rely on a 3rd party like github. Gerrit seems to do it slightly > differently with side by side diff's and patch revisions, but either > way would be an improvement. Please take this for what it's worth - I'm not a code reviewer or committer - just a pretty heavy user, and I lurk on (most?) of the mailing lists. Mostly I find bugs and ask others to fix them, since I lack the necessary intimate knowledge of postgresql internals to produce a meaningful patch. That said, I believe that - from my perspective - having postgresql's interaction with it's *large* community would only be improved by using something like github. I am far more likely to try to introduce a new feature, minor bugfix, code improvement, et cetera when using github than I would be if the interaction starts with a post to a mailing list and at least /looks/ like it might involve rather more than that. -- Jon
On Fri, Feb 8, 2013 at 2:23 AM, Magnus Hagander <magnus@hagander.net> wrote: > On Fri, Feb 8, 2013 at 1:32 AM, Josh Berkus <josh@agliodbs.com> wrote: >> This is a few too many steps, and certainly appears completely broken to >> any newcomer. > > I agree it's way too many step. Several of those can certainly be made > more efficient now that we have a more sane archives, well within the > scope of the current system. I have thought it'd be 'nice' if pre-generated binaries and git repos were made for each submitted patch. Is there a nice-and-tidy way to paginate over list traffic from an external program over The Internet?Is there/could there be a nice pagination marker? Isthat a reasonable step zero to making other things that consume email? -- fdr
On 8 February 2013 10:23, Magnus Hagander <magnus@hagander.net> wrote: >> to solve two problems: >> >> 1. maximize the efficiency of existing reviewer time >> >> 2. make tooling not be an obstacle to getting new reviewers > > I think you are missing a fundamental part in this - which is "0. > don't negatively affect the efficiency of existing committer time". Agreed. Process improvement is very important, but not more important than the process itself. What is the developer meeting for if not this discussion? Let's do some patch reviews and commits now and save the lets-make-it-better stufff for later. -- Simon Riggs http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services