Re: [libreoffice-projects] [ANN] Please use Gerrit from now on for Patch Review
On Fri, Jun 22, 2012 at 01:13:50PM +0200, Lionel Elie Mamane wrote: > No. When I have some free / floating time, I hunt for low-hanging > fruit in the review queue (patches I can review without understanding > the area), so that the "big" reviewers can focus on the more > complicated reviews. So restricting to a subarea is not helpful. Valid usecase indeed. > (But again, if it is low-hanging fruit, it won't take the big > reviewers much time anyway :) ) Oh, wouldnt be to certain of that. Best, Bjoern ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [libreoffice-projects] [ANN] Please use Gerrit from now on for Patch Review
On Thu, Jun 21, 2012 at 10:06:28AM +0200, Bjoern Michaelsen wrote: > On Thu, Jun 21, 2012 at 07:53:42AM +0200, Lionel Elie Mamane wrote: >> As I review very few patches, keeping me happy in this respect is >> probably not high priority, except maybe as a long tail argument (if >> we have 100 committers reviewing one patch per two months, that's >> still 600 reviews per year... Worth having for the project). > For 6 changes per year, wouldnt you already be well served with the > "watched projects"? No. When I have some free / floating time, I hunt for low-hanging fruit in the review queue (patches I can review without understanding the area), so that the "big" reviewers can focus on the more complicated reviews. So restricting to a subarea is not helpful. (But again, if it is low-hanging fruit, it won't take the big reviewers much time anyway :) ) -- Lionel ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [libreoffice-projects] [ANN] Please use Gerrit from now on for Patch Review
Hi Kendy, On Thu, Jun 21, 2012 at 11:25:18AM +0200, Jan Holesovsky wrote: > I believe this way we might keep both camps ("everything into ML" like > me, and "only discussions on the ML" like Bjoern) happy - because the > people who want to have only discussions on the ML would be able to > filter out messages based on the X-gerrit-review: header easily. Not really, as I am not concern for myself here (or anyone a bit more involved in the project, who thus has some kind of fancy filtering setup), but people who are newcomer/volunteers and are invested in the project vacational. Tuning the defaults of the notifications on the mailing list to those who are heavily involved (and have custom tooling, filtering etc. in place) would be wrong. These guys will always be able to opt-in or -out of any information relevant to them anyway . So the list should give a good place for vocational contributors by default and then be tweakable (by opt-in) to be the truckload-of-traffic-because-I-know-my-filters-monster only second. Best, Bjoern ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [libreoffice-projects] [ANN] Please use Gerrit from now on for Patch Review
Hi, On 2012-06-21 at 10:55 +0200, d.ostrov...@idaia.de wrote: > > Something like: > > - a short dialy digest of changes to keep reviewers in the loop > > - _one_ mail once a change goes in with all the comments/revisions and > > back-and-forth for this change in context in it > > > > or something completely different? We might get rid of the first (or > > only send > > it if there are changes untouched for more than a day) later or never do the > > second or tweak all of that, but for now we need a sensible start ;) > > > Sensible start would be: > 1. set up a new ML: Libreoffice-gerrit (analogical to LO-COMMIT ML) > 2. subscribe dev ML to get a daily digest from the 1. (or generate the > digest manually). > 3. set up IRC Bot to #libreoffice channel to notificate about gerrit events. So, me myself I'd like to get the gerrit notices going to the main mailing list, because I still want the current workflow (patches being sent to the development mailing list) possible. If one gets it from the ML, and pushes it, that's it. Though, for the review process via gerrit, anybody should be able to forward it to some rev...@gerrit.libreoffice.org or something (as already discussed in the other part of the thread, IIRC) that will just take the git format-patch's output from the mail, and apply it in gerrit. Gerrit then should send mail back to the ML + the author with the References: and In-reply-to: headers set up correctly so that it is a threaded answer to the original mail, informing the patch author in a friendly way that the patch has been applied in gerrit, and is pending for review. All the subsequent answers, should again go to the ML, again with the right References: and In-reply-to:, to keep threading. Of course - the gerrit mails should set their own X-gerrit-review: (or what) header so that people who do not want to see them in their libreoffice@ ML folder would be able to filter that out. I believe this way we might keep both camps ("everything into ML" like me, and "only discussions on the ML" like Bjoern) happy - because the people who want to have only discussions on the ML would be able to filter out messages based on the X-gerrit-review: header easily. Regards, Kendy ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [libreoffice-projects] [ANN] Please use Gerrit from now on for Patch Review
Zitat von Bjoern Michaelsen : Something like: - a short dialy digest of changes to keep reviewers in the loop - _one_ mail once a change goes in with all the comments/revisions and back-and-forth for this change in context in it or something completely different? We might get rid of the first (or only send it if there are changes untouched for more than a day) later or never do the second or tweak all of that, but for now we need a sensible start ;) Sensible start would be: 1. set up a new ML: Libreoffice-gerrit (analogical to LO-COMMIT ML) 2. subscribe dev ML to get a daily digest from the 1. (or generate the digest manually). 3. set up IRC Bot to #libreoffice channel to notificate about gerrit events. Regards David ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [libreoffice-projects] [ANN] Please use Gerrit from now on for Patch Review
Hi Winfried! On Thu, Jun 21, 2012 at 08:26:09AM +0200, Winfried Donkers wrote: > Not wanting to interfere, just to provide some feedback: > being a volunteer and being on the brink of newcomer and not-quite newcomer, > the mailing list gives me a lot of information. Comments on submitted patches > can be very informative for me. And you may know that information overload is > something that I suffer from more than others. So, from your point of view, what would be a good balance between information overflow and keeping the (valuable) comments on submitted patches? So what should be the 'default forced notification' to the list? Something like: - a short dialy digest of changes to keep reviewers in the loop - _one_ mail once a change goes in with all the comments/revisions and back-and-forth for this change in context in it or something completely different? We might get rid of the first (or only send it if there are changes untouched for more than a day) later or never do the second or tweak all of that, but for now we need a sensible start ;) Best, Bjoern ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [libreoffice-projects] [ANN] Please use Gerrit from now on for Patch Review
On Thu, Jun 21, 2012 at 07:53:42AM +0200, Lionel Elie Mamane wrote: > What I fear the most in that is that I have no way to mark a patch as > "I won't review it, not my area / I don't know / don't understand / > ...". With publish-to-ML, I just mark the post / whole thread as > "read". With gerrit, I fear I will see the same patch ever and ever > again in my queries... The web UI allows you to "star" a patch, and that is queryable from the commandline, but I dont know if it can be set from the commandline. > As I review very few patches, keeping me happy in this respect is > probably not high priority, except maybe as a long tail argument (if > we have 100 committers reviewing one patch per two months, that's > still 600 reviews per year... Worth having for the project). For 6 changes per year, wouldnt you already be well served with the "watched projects"? You can select in gerrit preferences to "watch" a project and get emails for each an every change in it (which is pretty much as good as a mailing list). But you can already filter there for changes touching certain files matching a regex e.g. ^connectivity/.*$ in addition(*), which I personally consider a killer feature. I guess you can find your 6 patches a year a lot easier with that than with one catch-all list (and if you did not review a patch after a 2 month, you just have a look at what is open) That said: registering a bot at gerrit with the mailing list as email and make it watch all of core should be rather trivial (once the mailing list is set up). Best, Bjoern (*) although there are currently some limitations on the file path filtering -- as gerrit seems to check those only when a change is uploaded, so you cant interactively tweak them. I hope gerrit will fix this to get so you can tweak this an get results "live". ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [libreoffice-projects] [ANN] Please use Gerrit from now on for Patch Review
On Thu, Jun 21, 2012 at 02:10:05AM -0500, Norbert Thiebaud wrote: > On Thu, Jun 21, 2012 at 12:53 AM, Lionel Elie Mamane wrote: >> What I fear the most in that is that I have no way to mark a patch as >> "I won't review it, not my area / I don't know / don't understand / >> ...". > No but... > 1/ you can 'star' patch > 2/ patch are presented in reverse chronological order > so you can scan the list of open... (...) > the next day the top of the pile will be new patche.. you scan until > you get to stuff you already saw (...) Ah yes, that's workable. -- Lionel ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [libreoffice-projects] [ANN] Please use Gerrit from now on for Patch Review
On Thu, Jun 21, 2012 at 12:53 AM, Lionel Elie Mamane wrote: > > What I fear the most in that is that I have no way to mark a patch as > "I won't review it, not my area / I don't know / don't understand / > ...". No but... 1/ you can 'star' patch 2/ patch are presented in reverse chronological order so you can scan the list of open... start the one you thing are 'interesting' the next day the top of the pile will be new patche.. you scan until you get to stuff you already saw and again 'star' stuff you care then, to concentrated on the one you stared you add 'is:stared' in the search filed (top right). Norbert ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [libreoffice-projects] [ANN] Please use Gerrit from now on for Patch Review
On 06/21/2012 08:08 AM, Norbert Thiebaud wrote: On Wed, Jun 20, 2012 at 11:57 PM, David Ostrovsky wrote: [...] As I explained on IRC: someone that _is_ a Committer can do some modification and still push the patch with you as author and him as commiter (git allow that, if we used svn like some other Indians, your scenario - author does not appear in the log unless he is the commiter - would be the norm. a given patch can have only one author... you don't get shared credit on a single patch... the alternative is to push a broken patch and then another patch to correct it... that is pushing breakage that render bisection very painful only to be pedant about 'authorship'. no thanks You said: "it can not be solved with gerrit: only i can change my gerrit patch/change." So it's probably more of an "it cannot be solved with git" (evolving a patch in discrete steps and having information about the discrete steps recorded in the final git history, yet hiding the steps in a single unit for purposes of bisecting; one longs for such a feature occasionally). As long as the standard gerrit workflow is to automatically preserve the initial patch's authorship when pushing it (whereas having to manually add comments about additional person's polishing activities into the git commit message is acceptable, as with the current patch-on-ML workflow), this is IMO OK. Stephan ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [libreoffice-projects] [ANN] Please use Gerrit from now on for Patch Review
On 06/20/2012 10:27 PM, David Ostrovsky wrote: On 20.06.2012 14:11, Stephan Bergmann wrote: On 06/19/2012 09:32 PM, David Ostrovsky wrote: I got one question with gerrit so far: how can other people contribute code snippet into foreign gerrit patch (so called extend it)? During my work on gbuildi'fication of pyuno module Stefan helped me with some scp2, Windows and Mac OS X specific stuff. But he can not put a change set into my gerrit patch. So he created a couple of patches and sent it to ML, I applied the patches and pushed the next iteration to gerrit. To be honest, the main reason I just dumped my changes onto the ML is that I couldn't get comfortable with the gerrit web UI. But hopefully the command line (which I haven't started to use yet) will suite me better... May be I'm missing something obvious here, but how would it change the things if you would use command line instead of web UI? The problem you brought up ("contribute code snippet into foreign gerrit patch") would likely not change at all with web vs cl. My remark was merely meant as a general rant against "yet another idiosyncratic web UI." ;) Stephan ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
RE: [libreoffice-projects] [ANN] Please use Gerrit from now on for Patch Review
> > Still, this removes the comments from many people's (potential) sight. > > The IMO big advantage of the "everything on a single mailing list" > > approach is that everybody is forced ;) to see everything (modulo > > information overload) > > So, IMHO that advantage not only has its drawbacks (information overload), > it is also mostly an illusion. Peoples forced _potential_ sight isnt really > helping > us here at all. > And the drawback (information overload) is hitting those hardest, who have > not finetuned their procmail for month/years -- read: newcomers and > volunteers. Not wanting to interfere, just to provide some feedback: being a volunteer and being on the brink of newcomer and not-quite newcomer, the mailing list gives me a lot of information. Comments on submitted patches can be very informative for me. And you may know that information overload is something that I suffer from more than others. I also learn a lot about gerrit and change-processes in groups, by the way :) All the best, Winfried ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [libreoffice-projects] [ANN] Please use Gerrit from now on for Patch Review
On Wed, Jun 20, 2012 at 11:57 PM, David Ostrovsky wrote: > While claiming other people's work to be your own may be not a problem in > other contries, > here in gemany it is: in fact minister of defence and other politicians > stepped down for doing exactly that (copy/paste parts of their dissertation > in that case). This is software development... not Academic papers. > And i would really like to see an commit author's face, if reviewer would > say: > Hey dude, i just entered some more comments to clarify what you have exactly > done in your 10.000 changed line patch and > have promoted it to repo ... with my user as author! > This is obviously a no go. Then fix your own patch... As I explained on IRC: someone that _is_ a Committer can do some modification and still push the patch with you as author and him as commiter (git allow that, if we used svn like some other Indians, your scenario - author does not appear in the log unless he is the commiter - would be the norm. a given patch can have only one author... you don't get shared credit on a single patch... the alternative is to push a broken patch and then another patch to correct it... that is pushing breakage that render bisection very painful only to be pedant about 'authorship'. no thanks You said: "it can not be solved with gerrit: only i can change my gerrit patch/change." I illustrated that this assertion was false. I did not suggest that it was the preferred way to do it, and as a matter of fact 'Committer' have a way to do it more nicely by preserving the 'author' information while correcting the patch for them. > > To make the things right and preserve the gerrit patch to be repo-pushable > all the time, > you have to conduct even more severe civil crime: to forge other people's > identity ;-) We do that all the time with patch we collect from the ML... very often the commtiter 'polish' the patch and still credit the original author for the whole thing.. the alternative is to reject the patch and tell the author to come back with a fixed version... not exactly the kind of dev-friedlyness we are aiming at. > but then you would definitelly be put in prison for that ... and you said > that you wanted visit LO congress this year ... go ahead, sue me. Me, and every copy-editor of every german newspaper... Norbert ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [libreoffice-projects] [ANN] Please use Gerrit from now on for Patch Review
On Wed, Jun 20, 2012 at 02:34:13PM +0200, Bjoern Michaelsen wrote: > On Wed, Jun 20, 2012 at 02:11:31PM +0200, Stephan Bergmann wrote: >> Still, this removes the comments from many people's (potential) >> sight. The IMO big advantage of the "everything on a single mailing >> list" approach is that everybody is forced ;) to see everything >> (modulo information overload) > I can assure you that I am not forced to see your comments on the mailing > list. Indeed, unless I am on CC or the subject sounds very thrilling the mail > body never passes my eye. Exactly: this is not about *forcing* people to see, but about a "push" model rather than a "pull" model. With publish-to-ML, it is "push" and we can "easily" (for some value of "easily") sort out what to look at from the subject line. Gerrit seemingly gets us to a "pull" model; we have to go look for patches to review, be it on the web interface or by command-line query. It becomes a separate action from reading the ML. What I fear the most in that is that I have no way to mark a patch as "I won't review it, not my area / I don't know / don't understand / ...". With publish-to-ML, I just mark the post / whole thread as "read". With gerrit, I fear I will see the same patch ever and ever again in my queries... And the "gerrit daily digest" idea floating around is not really, really helpful there, exactly because we lose the capacity of scanning for interesting patches by subject and the capacity to individually mark patches as "not to be looked at again". We already have separate libreoffice-commits and libreoffice-bugs mailing lists so as not to flood the "main" development mailing list with those; maybe we could use a "libreoffice-review" mailing list to get *one* mail for each gerrit action? Gerrit action = +1/-1 validate, +1/+2/-1/-2 codereview, gerrit automatic push, comment added, patch changed, new patch, ... And all emails concerning one patch/change in one thread, just like the bugzilla mails for one bug are all in one thread. I'd subscribe to such a mailing list (and obviously quickly scan by subject and not read most threads). As I review very few patches, keeping me happy in this respect is probably not high priority, except maybe as a long tail argument (if we have 100 committers reviewing one patch per two months, that's still 600 reviews per year... Worth having for the project). -- Lionel ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [libreoffice-projects] [ANN] Please use Gerrit from now on for Patch Review
On 20.06.2012 22:47, Norbert Thiebaud wrote: On Wed, Jun 20, 2012 at 3:27 PM, David Ostrovsky wrote: AFAIKs it can not be solved with gerrit: only i can change my gerrit patch/change. really ? I just did exactly that onhttps://gerrit.libreoffice.org/#/c/229/ Wow! no bad, not bad, but now this patch is not pushable any more (in this state) to repo, because by doing so it would preserve your identity (because the last change set always win). You have simple hijacked it ;-) While claiming other people's work to be your own may be not a problem in other contries, here in gemany it is: in fact minister of defence and other politicians stepped down for doing exactly that (copy/paste parts of their dissertation in that case). And i would really like to see an commit author's face, if reviewer would say: Hey dude, i just entered some more comments to clarify what you have exactly done in your 10.000 changed line patch and have promoted it to repo ... with my user as author! This is obviously a no go. To make the things right and preserve the gerrit patch to be repo-pushable all the time, you have to conduct even more severe civil crime: to forge other people's identity ;-) In your sample you have to push a new change set under Björn's (!) user (or Stephan under mine in my question): https://gerrit.libreoffice.org/#/c/229/ but then you would definitelly be put in prison for that ... and you said that you wanted visit LO congress this year ... ;-) Regards David ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [libreoffice-projects] [ANN] Please use Gerrit from now on for Patch Review
foreword: please trim the quotation when you reply... On Wed, Jun 20, 2012 at 3:27 PM, David Ostrovsky wrote: > > May be I'm missing something obvious here, but how would it change the > things if you would use command line instead of web UI? Choice is a great thing. > AFAIKs it can not be solved with gerrit: only i can change my gerrit > patch/change. really ? I just did exactly that on https://gerrit.libreoffice.org/#/c/229/ > The only way i can think of: you would have to create your own gerrit patch > and make it depends on my. > But then tinderboxes must know, that these two patches *must* be chained > together to be successfully verified. gerrit has the notion of 'depend' on and the tinderbox _can_ be taught about that, but really there is no reason to get unbuildable commit in On of the main advantage of gerrit 'review' workflow is that stuff can be rewritten even after having been 'published' for review... if a patch is borked, then it need to be amended. pushing a patch on top of it to fix it is ugly and render bisection pretty hard. Norbert ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [libreoffice-projects] [ANN] Please use Gerrit from now on for Patch Review
On 20.06.2012 14:11, Stephan Bergmann wrote: On 06/19/2012 09:32 PM, David Ostrovsky wrote: On 19.06.2012 19:24, Petr Mladek wrote: Sounds good but how many people would know about the comments? How hard would be to find them? https://gerrit.libreoffice.org/#/c/179/4/ (may be you need to login into gerrit with your openId) You can see it immediatelly: if and how much and for wich file exactly. For me this is one of the most valuable features of gerrit: inline comments. On comment column from 17 files Michael has commented in 5 files. This is already really good, isnt't? But it going to be even better: the submitter can respond (and he surely will, if he doesn't understand what the reviewer meant): in the context of this file/line. Still, this removes the comments from many people's (potential) sight. The IMO big advantage of the "everything on a single mailing list" approach is that everybody is forced ;) to see everything (modulo information overload), so that e.g. a comment given on one contributor's patch is picked up "by osmosis" by other contributors too (so one would hope). I know there is no golden road to spreading information most effectively, but I personally tend to prefer spreading/consuming too widely over too narrowly. I got one question with gerrit so far: how can other people contribute code snippet into foreign gerrit patch (so called extend it)? During my work on gbuildi'fication of pyuno module Stefan helped me with some scp2, Windows and Mac OS X specific stuff. But he can not put a change set into my gerrit patch. So he created a couple of patches and sent it to ML, I applied the patches and pushed the next iteration to gerrit. To be honest, the main reason I just dumped my changes onto the ML is that I couldn't get comfortable with the gerrit web UI. But hopefully the command line (which I haven't started to use yet) will suite me better... May be I'm missing something obvious here, but how would it change the things if you would use command line instead of web UI? AFAIKs it can not be solved with gerrit: only i can change my gerrit patch/change. The only way i can think of: you would have to create your own gerrit patch and make it depends on my. But then tinderboxes must know, that these two patches *must* be chained together to be successfully verified. This flow can be trivially simulated with native git command with patches: i will send you an almoust ready patch and would ask: Could you please take care of scp2 and Mac OS X specific stuff in my patch? You would just do: git am # my patch fix some stuff: git -am commit ... make && make dev-install and then you would push my and your changes *together*. Note: master is green all the time (!), you commited your changes under your own user (!) and i do not have to mess around with scp2 and friends ;-) As a casual contributor with small free time window i'm looking for a solution for this flow with gerrit. Regards David ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [libreoffice-projects] [ANN] Please use Gerrit from now on for Patch Review
On Wed, Jun 20, 2012 at 02:11:31PM +0200, Stephan Bergmann wrote: > Still, this removes the comments from many people's (potential) > sight. The IMO big advantage of the "everything on a single mailing > list" approach is that everybody is forced ;) to see everything > (modulo information overload) I can assure you that I am not forced to see your comments on the mailing list. Indeed, unless I am on CC or the subject sounds very thrilling the mail body never passes my eye. So, IMHO that advantage not only has its drawbacks (information overload), it is also mostly an illusion. Peoples forced _potential_ sight isnt really helping us here at all. And the drawback (information overload) is hitting those hardest, who have not finetuned their procmail for month/years -- read: newcomers and volunteers. Best, Bjoern ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [libreoffice-projects] [ANN] Please use Gerrit from now on for Patch Review
On 06/19/2012 09:32 PM, David Ostrovsky wrote: On 19.06.2012 19:24, Petr Mladek wrote: Sounds good but how many people would know about the comments? How hard would be to find them? https://gerrit.libreoffice.org/#/c/179/4/ (may be you need to login into gerrit with your openId) You can see it immediatelly: if and how much and for wich file exactly. For me this is one of the most valuable features of gerrit: inline comments. On comment column from 17 files Michael has commented in 5 files. This is already really good, isnt't? But it going to be even better: the submitter can respond (and he surely will, if he doesn't understand what the reviewer meant): in the context of this file/line. Still, this removes the comments from many people's (potential) sight. The IMO big advantage of the "everything on a single mailing list" approach is that everybody is forced ;) to see everything (modulo information overload), so that e.g. a comment given on one contributor's patch is picked up "by osmosis" by other contributors too (so one would hope). I know there is no golden road to spreading information most effectively, but I personally tend to prefer spreading/consuming too widely over too narrowly. I got one question with gerrit so far: how can other people contribute code snippet into foreign gerrit patch (so called extend it)? During my work on gbuildi'fication of pyuno module Stefan helped me with some scp2, Windows and Mac OS X specific stuff. But he can not put a change set into my gerrit patch. So he created a couple of patches and sent it to ML, I applied the patches and pushed the next iteration to gerrit. To be honest, the main reason I just dumped my changes onto the ML is that I couldn't get comfortable with the gerrit web UI. But hopefully the command line (which I haven't started to use yet) will suite me better... Stephan ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [libreoffice-projects] [ANN] Please use Gerrit from now on for Patch Review
Hi Tor, On Tue, Jun 19, 2012 at 10:14:27PM +0300, Tor Lillqvist wrote: > But if the intent is that *all* changes are to go through gerrit, Its not. As said repeatedly on this thread already, everyone who has an fd.o account will be able to continue to push to master. However, the hope is that the convenience of having a windows build before it hits master and being able to fix it without time pressure and 20 angry other devs shouting at you on IRC will convince you to use it for more and more stuff. > surely the majority of changes (number-of-lines-wise, not number-wise) going > through it will be feature work and cleanups, not patches? Once we tune the tinderboxes to build stuff submitted to review there, having these build will be quite nice esp. for features etc. And if your patch is in danger of rotting away with nobody giving it love, the tinderboxes where happy with it, you can still push it directly to master on you own risk. But even there, I think asking another dev on IRC "hey Im confident with that, the tinderboxes are fine with it, could you give it a quick review otherwise I would push myself" would give you what you want. Its all checks and balances: Nobody will be powerplayed and not get his stuff in, but there still might someone else at least skimming over it in addition now. Its a bit different for those who do not have direct commit access -- they can only hope for somebody reviewing and pushing their change. But that is also not different from before for them. If anything changes, its that we can give them direct commit access quicker and with less hassle now. OTOH, the hope is, that the urgent need for that is shrinking too anyway. > Will this mean people will start doing less refactoring cleanups, for > instance, in order to make their change sets smaller, to increase the > possibility of somebody reviewing the, eh, "patch"? No. Best, Bjoern ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [libreoffice-projects] [ANN] Please use Gerrit from now on for Patch Review
Hi Petr, all, i am using gerrit for a while now and gathered some experience with it already and would like to share it with you. On 19.06.2012 19:24, Petr Mladek wrote: Bjoern Michaelsen píše v Út 19. 06. 2012 v 18:40 +0200: Hi Petr, On Tue, Jun 19, 2012 at 06:14:18PM +0200, Petr Mladek wrote: Ah, this bug is about a daily digest. I think that we first need to decide how much we want to modify the current work flow. Do we want to really move most discussions from the mailing list to gerrit? IMHO yes, they are noise on the mailing list as you need deep context to follow them. In gerrit you can even comment inline in the patch. Sounds good but how many people would know about the comments? How hard would be to find them? https://gerrit.libreoffice.org/#/c/179/4/ (may be you need to login into gerrit with your openId) You can see it immediatelly: if and how much and for wich file exactly. For me this is one of the most valuable features of gerrit: inline comments. On comment column from 17 files Michael has commented in 5 files. This is already really good, isnt't? But it going to be even better: the submitter can respond (and he surely will, if he doesn't understand what the reviewer meant): in the context of this file/line. Still not convinced? How about this: 12 comments from different peoples on only one line? https://gerrit-review.googlesource.com/#/c/35380/3/gerrit-server/src/main/java/com/google/gerrit/server/project/ListProjects.java You can try to simulate this flow in mail with patch(es)... How people will be noticed about news in gerrit? Reporters and Reviewer will get mails about "their" patches. Unless they prefer a different workflow in which case they can change their preferences. A lot better than what we have now. Sure but who will be the reviewer? In my case (gbuild'ification of dmake module) the usual suspects are Stefan, David Tardon, Michael Stahl, Matus and Björn. ;-) I guess every LO-area has their own specalists. The mailing list has the advantage that people step in when they are interested. I agree with you as far as globally interested topics are discussed. But with my (build specific) questions only build experts were able to help. But then the context was wrong anyway: Mail to ML with [GERRIT] in topic or [PATCH] for that matter and reall special questions in the body, like how to fix broken pyuno bridge on mingw platform (because no python is currently get packaging in the installation set there). Those mail created noise on the ML, but those two people who answered were already on the CC list in the mentioned gerrit patch. So I would better just commented something there (inline) and they would be notify anyway and respond to me (without noise) and in the context of this gerrit patch. Another most valuable gerrit feature for me is the preserving of patch history (many change sets on gerrit) and still having only one commit in the real repository. This magic is achived through the combination of ChangeId git hook, git rebase -i command and gerrit's handling of ChangeId as the context of gerrit patch. This flow may be not so obvious: git commit # some changes (git ChangeId hook generate a fresh ChangeId) git push logerrit HEAD:refs/for/master # gerrit patch with first change set is created. git commit # further changes git rebase -i HEAD~2 # with fixup preserves the same ChangeId git push logerrit HEAD:refs/for/master # second change set in the same (!) gerrit patch is created (same ChangeId) And the best is: you can see the evolution of the patch (with command line tool, gerrit web interface or gitweb/cgit). I got one question with gerrit so far: how can other people contribute code snippet into foreign gerrit patch (so called extend it)? During my work on gbuildi'fication of pyuno module Stefan helped me with some scp2, Windows and Mac OS X specific stuff. But he can not put a change set into my gerrit patch. So he created a couple of patches and sent it to ML, I applied the patches and pushed the next iteration to gerrit. While is was working, this flow obviously violates the common repository rule: every contribution should hit repository with the name of the right contributor. Sure we could create a new integration branch, like lately gbuild_merge branch and push our own gerrit patches there... Ideas on this? Regards David ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [libreoffice-projects] [ANN] Please use Gerrit from now on for Patch Review
I find it peculiar that in the discussion here people keep talking about patches, as if gerrit was only a "patch" review tool. At least I understand "patch" to mean a relatively local change to the code in order to fix some specific bug. But if the intent is that *all* changes are to go through gerrit, surely the majority of changes (number-of-lines-wise, not number-wise) going through it will be feature work and cleanups, not patches? Will this mean people will start doing less refactoring cleanups, for instance, in order to make their change sets smaller, to increase the possibility of somebody reviewing the, eh, "patch"? --tml ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [libreoffice-projects] [ANN] Please use Gerrit from now on for Patch Review
Hi, On Tue, Jun 19, 2012 at 07:24:30PM +0200, Petr Mladek wrote: > Sure but who will be the reviewer? The mailing list has the advantage > that people step in when they are interested. It helps to balance the > workload. Also it is very open for new reviewers. I am afraid that > gerrit could make some people fell like reviewing robots. There is no "default reviewer", so patches would just wait for somebody to pick them up, just like on the list. Add just like you can CC someone on the list, you can set a reviewer when submitting the patch. But in addition someone looking into the submitted change can say "I have no idea about this area of code, but foo might", just like on the list, but with less noise. If some devs get to be reviewing robots that is unfortunate and we would need to find a solution, but I doubt that to be related to gerrit, it would be the same on the list -- just with more noise. Note that you can watch for changes matching some criteria: http://gerrit.googlecode.com/svn/documentation/2.1.6/user-search.html and get an email, when one of those arrives. So you can get an email for patches that touch "^sw/" or "^sc/" without having to wade through the rest. I know it takes courage to self-inflict yourself to be spammed for incoming patches, but if we core devs do that with changes we feel confortable about, we can make life easier for everyone, as there hopefully will be only few patches to review left after everyone takes care about the ones he feels comfortable about (and we shorten a lot of the "I dont know about that one, foo care to take a look?" noise). It makes you make your quota of reviews with patches you actually care about, which is kinda nice, I guess. > Let's discuss this on the ESC meeting. I am afraid that people will not > look into gerrit regularly. For example, I open bugzilla only when I get > a mail about a change or when I see an interesting bug number in mail > conference or irc. > > People have only limited time to monitor mailing lists, irc, other > tools. We need to be careful about adding too many channels to monitor. see above, I guess a daily digest to the ml and a watch on the modules you care about will get better focused information for everyone. Also note that some people might prefer to browse the patches on gerrit. I for one would prefer to wade through some patches en bloc in the morning and then proceed to the interesting discussions on the dev list rather than having both sprinkled into each other. Best, Bjoern ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [libreoffice-projects] [ANN] Please use Gerrit from now on for Patch Review
Bjoern Michaelsen píše v Út 19. 06. 2012 v 18:40 +0200: > Hi Petr, > > On Tue, Jun 19, 2012 at 06:14:18PM +0200, Petr Mladek wrote: > > Ah, this bug is about a daily digest. I think that we first need to > > decide how much we want to modify the current work flow. Do we want to > > really move most discussions from the mailing list to gerrit? > > IMHO yes, they are noise on the mailing list as you need deep context to > follow > them. In gerrit you can even comment inline in the patch. Sounds good but how many people would know about the comments? How hard would be to find them? > > How people will be noticed about news in gerrit? > > Reporters and Reviewer will get mails about "their" patches. Unless they > prefer > a different workflow in which case they can change their preferences. A lot > better than what we have now. Sure but who will be the reviewer? The mailing list has the advantage that people step in when they are interested. It helps to balance the workload. Also it is very open for new reviewers. I am afraid that gerrit could make some people fell like reviewing robots. > > Is one day digest really enough? > > For the mailing list, yes. But I might make sense to teach one of our IRC bots > to blurp out updates in realtime to #libreoffice-dev. > One might consider it useful to send _one_ mail to the list once the patch > goes > in with a summary of the history, but I personally guess the majority is not > interested in reading that and the few that are can look it up in gerrits web > interface. Let's discuss this on the ESC meeting. I am afraid that people will not look into gerrit regularly. For example, I open bugzilla only when I get a mail about a change or when I see an interesting bug number in mail conference or irc. People have only limited time to monitor mailing lists, irc, other tools. We need to be careful about adding too many channels to monitor. > Well, we should still try to get people on the mailing list (and it will be > easier with less traffic there). Asking a first-time contributor to subscribe > (and send his license blurp) is a good thing. A lot of volunteers told me > though they unsubscribed because of the volume of mails on the dev list. And > the PATCH mails arent exactly those that are socially attractive. It sounds reasonable. The question is if people will still monitor the mailing list if there are not interesting patches. Well, I do not want to be too negative. We need to try it. On the other hand, we need to be ready to solve these problems ;-) > > + setup mail notifications to the developer mailing list > > Yes, and Robert is on that bug (ETA: next week). Otherwise, I will jump in. Sounds good > > + setup patch drop mailbox > yes, on my todo and halfway there: > https://launchpad.net/~r-gerrit-0 (bot-id) > ger...@libreoffice.org (bot email) > > Design proposals on the email format etc. welcome. I am sure that people will have many good ideas once they see how it is supposed to work. > > + scripting to automagically identify patches on the mailing > > list > > IMHO, we should drop that in favor of explicit forward to > ger...@libreoffice.org. "Guessing" the target branch is tricky, and a simple > forward (with possibly tweaking the subject to something botreadable) shouldnt > be too hard. If the gerrit commandline interface is easy, anyone would be able to push it for review quickly. We need easy interface anyway. > > For inspiration, I attach mail from the openSUSE Build Service. It > > includes: > > > > + description of changes > > + diff of the spec file > > + commands how to get full diff, approve or reject the change > > I still think we should reduce the traffic on the dev ml, but having that as > IRC bot would be nice. Such mails are kinda nice for full-time employees, but > for everyone else they are overkill IMHO. We must not increase the traffic. If active reviewers want notification, we need to provide it. Let's discuss this on ESC. Best Regards, Petr ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [libreoffice-projects] [ANN] Please use Gerrit from now on for Patch Review
Hi Petr, On Tue, Jun 19, 2012 at 06:14:18PM +0200, Petr Mladek wrote: > Ah, this bug is about a daily digest. I think that we first need to > decide how much we want to modify the current work flow. Do we want to > really move most discussions from the mailing list to gerrit? IMHO yes, they are noise on the mailing list as you need deep context to follow them. In gerrit you can even comment inline in the patch. > How people will be noticed about news in gerrit? Reporters and Reviewer will get mails about "their" patches. Unless they prefer a different workflow in which case they can change their preferences. A lot better than what we have now. > Is one day digest really enough? For the mailing list, yes. But I might make sense to teach one of our IRC bots to blurp out updates in realtime to #libreoffice-dev. One might consider it useful to send _one_ mail to the list once the patch goes in with a summary of the history, but I personally guess the majority is not interested in reading that and the few that are can look it up in gerrits web interface. > Also, you know, there is the "hello/thanks/see you" much more involved > in the mailing list. I think that such things are important for the > community live. Well, we should still try to get people on the mailing list (and it will be easier with less traffic there). Asking a first-time contributor to subscribe (and send his license blurp) is a good thing. A lot of volunteers told me though they unsubscribed because of the volume of mails on the dev list. And the PATCH mails arent exactly those that are socially attractive. > + setup mail notifications to the developer mailing list Yes, and Robert is on that bug (ETA: next week). Otherwise, I will jump in. > + setup patch drop mailbox yes, on my todo and halfway there: https://launchpad.net/~r-gerrit-0 (bot-id) ger...@libreoffice.org (bot email) Design proposals on the email format etc. welcome. > + scripting to automagically identify patches on the mailing > list IMHO, we should drop that in favor of explicit forward to ger...@libreoffice.org. "Guessing" the target branch is tricky, and a simple forward (with possibly tweaking the subject to something botreadable) shouldnt be too hard. > For inspiration, I attach mail from the openSUSE Build Service. It > includes: > > + description of changes > + diff of the spec file > + commands how to get full diff, approve or reject the change I still think we should reduce the traffic on the dev ml, but having that as IRC bot would be nice. Such mails are kinda nice for full-time employees, but for everyone else they are overkill IMHO. Best, Bjoern ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [libreoffice-projects] [ANN] Please use Gerrit from now on for Patch Review
Bjoern Michaelsen píše v Út 19. 06. 2012 v 13:38 +0200: > On Tue, Jun 19, 2012 at 11:13:45AM +0200, Petr Mladek wrote: > > It means that gerrit should be able to detect patches for review on the > > mailing list, integrate them, and make them ready for review. > > > > My expectation would be that it sends a replay to the mailing list with > > a link to diff, link to build results and commands to approve it. > > Please provide constructive feedback to for example: > > https://bugs.freedesktop.org/show_bug.cgi?id=51159 Ah, this bug is about a daily digest. I think that we first need to decide how much we want to modify the current work flow. Do we want to really move most discussions from the mailing list to gerrit? How people will be noticed about news in gerrit? Is one day digest really enough? Also, you know, there is the "hello/thanks/see you" much more involved in the mailing list. I think that such things are important for the community live. Note that the cooperation with mailing list is even in the plan, see: + setup mail notifications to the developer mailing list + setup patch drop mailbox + scripting to automagically identify patches on the mailing list at http://wiki.documentfoundation.org/Development/GerritMigration#Action_items I think that gerrit should send mail to the mailing list immediately when someone needs review (people without commit access, people that are not sure about their fix). For inspiration, I attach mail from the openSUSE Build Service. It includes: + description of changes + diff of the spec file + commands how to get full diff, approve or reject the change > > http://nabble.documentfoundation.org/gerrit-for-release-branches-bots-ftw-tp3990804.html I am going to answer it but I am busy with some other things. > implementing this "into the blind" just for it to be rejected by those who > where silent before is not the way to go. I think that my request is nothing new for you. I think that it was actually one of the most important requests since the beginning. Best Regards, Petr --- Begin Message --- home:fstrba/libcdr -> devel:libraries:c_c++/libcdr https://build.opensuse.org/request/show/124460 Description: New upstream release changes files: -- --- libcdr.changes +++ libcdr.changes @@ -1,0 +2,7 @@ +Mon Jun 11 14:32:55 CEST 2012 - fridrich.st...@suse.com + +- Update to upstream version 0.0.8 + * initial text support +- Remove upstreamed patch + +--- old: libcdr-0.0.7-clang.patch libcdr-0.0.7.tar.xz new: libcdr-0.0.8.tar.xz spec files: --- --- libcdr.spec +++ libcdr.spec @@ -28,13 +28,12 @@ BuildRequires: gcc-c++ pkgconfig xz zlib-devel liblcms2-devel BuildRequires: libwpd-devel >= 0.9.0 libwpg-devel >= 0.2.0 Summary:Library for parsing the Corel Draw file format structure -Version:0.0.7 +Version:0.0.8 Release:0 License:LGPLv2+, GPLv2+, MPL1.1 Group: Productivity/Publishing/Word Url:http://www.freedesktop.org/wiki/Software/libcdr Source: http://dev-www.libreoffice.org/src/libcdr-%{version}.tar.xz -Patch0: libcdr-0.0.7-clang.patch BuildRoot: %{_tmppath}/%{name}-%{version}-build %description @@ -86,7 +85,6 @@ %prep %setup -q -%patch0 -p1 %build %configure --disable-static --docdir=%_docdir/%name other changes: -- ++ libcdr-0.0.8.tar.xz (new) ++ deleted files: --- libcdr-0.0.7-clang.patch --- libcdr-0.0.7.tar.xz To REVIEW against the previous version: osc request show --diff 124460 To ACCEPT the request: osc request accept 124460 --message="reviewed ok." To DECLINE the request: osc request decline 124460 --message="declined for reason xyz (see ... for background / policy / ...)." To REVOKE the request: osc request revoke 124460 --message="retracted because ..., sorry / thx / see better version ..." -- Hermes messaging (http://hermes.opensuse.org) openSUSE Build Service (https://build.opensuse.org/) Collaboration: http://en.opensuse.org/Build_Service/Collaboration --- End Message --- ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [libreoffice-projects] [ANN] Please use Gerrit from now on for Patch Review
On Tue, Jun 19, 2012 at 11:13:45AM +0200, Petr Mladek wrote: > It means that gerrit should be able to detect patches for review on the > mailing list, integrate them, and make them ready for review. > > My expectation would be that it sends a replay to the mailing list with > a link to diff, link to build results and commands to approve it. Please provide constructive feedback to for example: https://bugs.freedesktop.org/show_bug.cgi?id=51159 and http://nabble.documentfoundation.org/gerrit-for-release-branches-bots-ftw-tp3990804.html implementing this "into the blind" just for it to be rejected by those who where silent before is not the way to go. Best, Bjoern ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [libreoffice-projects] [ANN] Please use Gerrit from now on for Patch Review
Bjoern Michaelsen píše v Po 18. 06. 2012 v 12:09 +0200: > Hi all, > > with: > > http://sweetshark.livejournal.com/13298.html > > gerrit is documented and ready to go. Ah, there are several strange and long commands. Also I miss the cooperation with the mailing list. I remember that the main request was that mailing list will stay the main communication point between developers. All patches that need review must be mentioned there. It means that gerrit should be able to detect patches for review on the mailing list, integrate them, and make them ready for review. My expectation would be that it sends a replay to the mailing list with a link to diff, link to build results and commands to approve it. Similar mail should be send for patches that were directly pushed into gerrit for review. Of course, it must not send mail for every single patch but just for patches where the developer explicitly asks for review, so the numbers of mails on the mailing list stays the same. > Please use it for code review as much as possible now as it simplifies > things a lot over manual patch fiddling on mailing lists. I will > update the EasyHacks to point to gerrit instead in the next days. IMHO, we first need to conclude that gerrit is in usable state. > The last remaining step will be making the repo at gerrit the reference (and > the one at freedesktop a read-only mirror). I assume that to be prepared and > done until mid-July(*). Same here. Best Regards, Petr PS: To make it clear. I am neutral about gerrit. I see some advantages and also some risks. In each case, we must make Caolan, Kendy, Michael, Stefan, Eike, Miklos, and all other very active patch reviewers happy with it. I am excited when I look at the current mailing list and see so many patches reviewed a pushed within one or two days. It helps to make contributors active. It helps me a lot to do the tag in time. We must not break this flow! ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice