[ANN] Please use Gerrit from now on for Patch Review

classic Classic list List threaded Threaded
90 messages Options
Next » 12345 « Prev
Bjoern Michaelsen Bjoern Michaelsen
Reply | Threaded
Open this post in threaded view
|

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
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Bjoern Michaelsen Bjoern Michaelsen
Reply | Threaded
Open this post in threaded view
|

Re: [ANN] Please use Gerrit from now on for Patch Review

In reply to this post by Lionel Elie Mamane
Hi,

On Thu, Jun 21, 2012 at 07:31:15AM +0200, Lionel Elie Mamane wrote:
> I understand gerrit is not able to "understand" such policies and we
> will continue to enforce them "manually" by giving only codereview+1
> unless there are already two other codereview+1.

But a bot using:

 http://gerrit.googlecode.com/svn/documentation/2.2.1/cmd-query.html
 http://gerrit.googlecode.com/svn/documentation/2.2.1/user-search.html
 http://gerrit.googlecode.com/svn/documentation/2.2.1/cmd-stream-events.html
 http://gerrit.googlecode.com/svn/documentation/2.2.1/cmd-gsql.html
 http://gerrit.googlecode.com/svn/documentation/2.2.1/cmd-review.html

is able to understand such policies and can execute the most weirdo custom
workflow we possible can imagine -- all without having to tweaking too much on
gerrit itself.

Best,

Bjoern
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Lionel Elie Mamane Lionel Elie Mamane
Reply | Threaded
Open this post in threaded view
|

Re: [ANN] Please use Gerrit from now on for Patch Review

In reply to this post by Bjoern Michaelsen
On Thu, Jun 21, 2012 at 09:46:54AM +0200, Bjoern Michaelsen wrote:
> On Thu, Jun 21, 2012 at 07:09:15AM +0200, Lionel Elie Mamane wrote:

>> But frankly, why should Google, AOL, Wordpress or another person be
>> able to impersonate me at the TDF systems?

> If you created an account at one of those, you are trusting
> them. The trust issue is with account creation, not with usage. Once
> you have a google account it is automatically OpenID enabled. Even
> if you never used it yourself, google is perfectly able to
> impersonate you.

No, if I create a gerrit account with a non-Google OpenID identity,
get it added to the right "privileged groups" (committer, can review,
can submit patches with different author, ...) and I have an
OpenID-enabled Google account, then Google is able to create a *new*
account at Gerrit with my Google identity with *no* more privileges
than we give any random person. It is *not* able (modulo security
issues in Gerrit or my other OpenID provider) to access my Gerrit
account, as long as I (or my OpenID provider or anybody cracking them)
don't go into my Gerrit account and link my Google-issued OpenID
identity to my Gerrit account.

> The same is true for an email/password-login and any external mail
> provider.

No, my email being hosted at gmail does not mean Google knows, or can
know, my username/password at wiki.documentfoundation.org; yes, they
can request a new password to be mailed and intercept it, but then
I'll notice something is wrong: I cannot login at the wiki anymore!
(For the specific case of google, they could put a spy feature in
Chrome, OK... like the author of about any software I use.)

--
Lionel
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Bjoern Michaelsen Bjoern Michaelsen
Reply | Threaded
Open this post in threaded view
|

Re: [libreoffice-projects] [ANN] Please use Gerrit from now on for Patch Review

In reply to this post by Winfried Donkers
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
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
d.ostrovsky d.ostrovsky
Reply | Threaded
Open this post in threaded view
|

Re: [libreoffice-projects] [ANN] Please use Gerrit from now on for Patch Review

Zitat von Bjoern Michaelsen <[hidden email]>:

>
> 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
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Michael Meeks-2 Michael Meeks-2
Reply | Threaded
Open this post in threaded view
|

Re: [ANN] Please use Gerrit from now on for Patch Review

In reply to this post by Bjoern Michaelsen

On Wed, 2012-06-20 at 22:46 +0200, Bjoern Michaelsen wrote:
> we vaguely considered running a TDF OpenID provider in the distant future,
> but so shied away from that for the nontrivial cost (security is hard to
> get right)

        I imagine if Lionel wanted to re-open that decision, and has done the
work anyway to get an openID server setup, -and- we labelled it clearly
as some "low security, for internal infrastructure only" thing - then
the sysadmin guys might be able to help get Lionel plugged into their
world to do that.

        I'd poke Thorsten Behrens / Florian Effenberger I guess if you'd be
interested in setting that up & maintaining it (personally).

        Failing that a hack-around might be to create an account with some
temporary openID identity, setup the ssh key (so the command-line tools
work), then having some server tweak to disable that user from weblogin.
That'd make you an automatic champion of command-line parity too I
guess ;-)

        HTH,

                Michael.

--
[hidden email]  <><, Pseudo Engineer, itinerant idiot

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Jan Holesovsky Jan Holesovsky
Reply | Threaded
Open this post in threaded view
|

Re: [libreoffice-projects] [ANN] Please use Gerrit from now on for Patch Review

In reply to this post by d.ostrovsky
Hi,

On 2012-06-21 at 10:55 +0200, [hidden email] 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 [hidden email] 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
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Bjoern Michaelsen Bjoern Michaelsen
Reply | Threaded
Open this post in threaded view
|

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
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Lionel Elie Mamane Lionel Elie Mamane
Reply | Threaded
Open this post in threaded view
|

Re: [ANN] Please use Gerrit from now on for Patch Review

In reply to this post by Michael Meeks-2
On Thu, Jun 21, 2012 at 10:13:38AM +0100, Michael Meeks wrote:
> On Wed, 2012-06-20 at 22:46 +0200, Bjoern Michaelsen wrote:

>> we vaguely considered running a TDF OpenID provider in the distant future,
>> but so shied away from that for the nontrivial cost (security is hard to
>> get right)

> I imagine if Lionel wanted to re-open that decision, and has
> done the work anyway to get an openID server setup,

In short: I've done the work for a small-scale OpenID server (from one
user to a few users, each user being configured manually in a text
file). The implementations I've looked at would most probably not be
adequate for a bigger setup like TDF. Security being one of my core
interests, if there would be interest in a TDF OpenID provider, I
could be interested in participating in its setup, but we'd probably
select a more "large scale" implementation that the ones I now have
experience with.

In particular, local-openid is intrinsically single-user; but one can
run multiple copies of it :) (that is partially a joke; running it on
a machine that anybody else than you has a shell account on has
security implications I'd need to think about how to resolve). Part of
its appeal is that it is not run "system-wide", but that the user that
wants to authenticate runs it hirself from a shell account.

The other implementation I've setup is SimpleID; that's the one where
each user is configured manually in a text file, but we can delegate
that to the user hirself through symlinks. Security-wise, the password
is stored as an *unsalted* hash, but that would be easy enough to
change should we want to.

--
Lionel
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Terrence Enger Terrence Enger
Reply | Threaded
Open this post in threaded view
|

Re: [ANN] Please use Gerrit from now on for Patch Review

In reply to this post by Bjoern Michaelsen
On Wed, 2012-06-20 at 22:46 +0200, Bjoern Michaelsen wrote:

> Hi,
>
> On Wed, Jun 20, 2012 at 09:47:48PM +0200, Lionel Elie Mamane wrote:
>
> > My point is basically that it is too much of an investment for a
> > casual contributor... If we could make that easier by allowing plain
> > username+password (or exporting bugzilla accounts over OpenID? I guess
> > that would be *more* work), I feel it would lower the barrier to entry
> > to gerrit.
>
> I think you are part of a very, very rare demographic there

By the way ...

There is a group of people--well, one person anyway--more concerned
with trusting *myself* to understand the daunting user agreements
presented by the OpenId providers.  I brought this up on the discuss
list in the sub-thread "Re: [tdf-discuss] Re: AskLibO blitzes"
<http://listarchives.documentfoundation.org/www/discuss/msg08774.html">.
The discussion in the sub-thread quickly moved back to the question of
users trusting the providers; that proves that my concern is not
widespread.

Terry.


_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Lionel Elie Mamane Lionel Elie Mamane
Reply | Threaded
Open this post in threaded view
|

Re: [ANN] Please use Gerrit from now on for Patch Review

On Thu, Jun 21, 2012 at 08:07:45AM -0400, Terrence Enger wrote:
> On Wed, 2012-06-20 at 22:46 +0200, Bjoern Michaelsen wrote:
>> On Wed, Jun 20, 2012 at 09:47:48PM +0200, Lionel Elie Mamane wrote:

>>> My point is basically that it is too much of an investment for a
>>> casual contributor... If we could make that easier by allowing
>>> plain username+password (or exporting bugzilla accounts over
>>> OpenID? I guess that would be *more* work), I feel it would lower
>>> the barrier to entry to gerrit.

>> I think you are part of a very, very rare demographic there

> By the way ...

> There is a group of people--well, one person anyway--more concerned
> with trusting *myself* to understand the daunting user agreements
> presented by the OpenId providers.  (...) the sub-thread quickly
> moved back to the question of users trusting the providers; that
> proves that my concern is not widespread.

I see your concern as a subset of "trusting the provider". You want to
read and understand the user agreement because you don't trust that
the user agreement is reasonable :)

--
Lionel
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Pierre-André Jacquod Pierre-André Jacquod
Reply | Threaded
Open this post in threaded view
|

Re: [ANN] Please use Gerrit from now on for Patch Review

In reply to this post by Bjoern Michaelsen
Hello,
well as free time contributor with commit access to current repository,
I followed this gerrit story. So not kind of surprise, but yeah, until
THIS mail and posts, was me very unclear what it would mean.

To say it, first I had a lot of doubt. Reading mails, entry in the wiki,
looking the web ans thinking, I now have the feeling this could be a
good move.

Just to say, I basically just really do not like the fact of having to
use openId, would prefer to have an account at fd.o. I did it really for
the LibO, a kind of forced to. And NO, I do NOT have any google, flickr,
facebook or other account. Do not trust them enough to put my data
there, so to use them as authentication.... But no choice (!) I kind of
understood. [+1 for / to Lionel thread]

I - sorry - do not want to read a lot gerrit documentation, I am here
already in my free time. So I raise some questions that are not clear
for me, that I did not saw in the wiki, based on my previous
experiences. Here some points:

[WORKFLOW]
1. I have been beaten back by tinderbox (Mac OS X hurt me well), fixing
some stupid point of my commits run after run. So would this workflow be
supported :
some patch done
I push to gerrit and let tinderbox run. In case of success I get ? a +1
of each tinderbox ? How do I know it passes with success.
Once tinderbox is successful, since my change is small or I am confident
with, I put a +2 to my patch, which means this will insert automatically
it to master. So I will be able to positive review my own patch ?

2. If I have several patches, that need to be together, how should I
proceed with pushing them together to gerrit ? (If I want to have three
patches in the same review ID from gerrit) ?

[ADMIN]
1. I followed the asked process of setting up an account, etc.. mailed
to Norbert so he can match my fd.o account to gerrit. Well, how may I
now see that I actually have / will have commit access to gerrit repo ?

2. When my ssh key expires, it is enough to just change it into my
gerrit account ?

3. May be a stupid remark, but why do not use this way (opening gerrit
account ) then to ask for license stuff ? You would have to ack to be
able to open account, or to load patches. This would ease, ensure the
process towards this goal ?

Thanks & Regards
Pierre-André

On 06/18/2012 12:09 PM, Bjoern Michaelsen wrote:

>
> Hi all,
>
> with:
>
>   http://sweetshark.livejournal.com/13298.html
>
> gerrit is documented and ready to go. 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.
> 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(*).
>>From that point on, we will have a lot of opportunity to improve our tinderbox
> testing and reporting, making life easier and better for everyone working on
> the codebase.
>
> Best,
>
> Bjoern
>
> (*) Along with the "other" repos.
> _______________________________________________
> LibreOffice mailing list
> [hidden email]
> http://lists.freedesktop.org/mailman/listinfo/libreoffice
>


_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Bjoern Michaelsen Bjoern Michaelsen
Reply | Threaded
Open this post in threaded view
|

Re: [ANN] Please use Gerrit from now on for Patch Review

Hi Pierre-André!

On Thu, Jun 21, 2012 at 07:08:46PM +0200, Pierre-André Jacquod wrote:
> I push to gerrit and let tinderbox run. In case of success I get ? a
> +1 of each tinderbox ? How do I know it passes with success.

a tinderbox would give you a +1verified, which means that it builds (and passes
whatever kind of tests that kind of tinderbox is gracious enough to do).

IMHO for LibreOffice we should use "verified" as meaning "couldnt find anything
evil by blackbox testing".

A tinderbox can never do "+1codereview" as a tinderbox doesnt understand code,
but it can blackbox test it (does it still compile and pass tests).

Btw, gerrit in theory allows more "labels" than "codereview" and "verified", so
we could have a "codebeauty" label or something -- and ignore it. Lets just
start with the default and only make things more complex when we really need
it.


IMHO, we should not use verified as "fixes the bug it claims to fix" -- thats
what bugzilla is for.

> Once tinderbox is successful, since my change is small or I am
> confident with, I put a +2 to my patch, which means this will insert
> automatically it to master. So I will be able to positive review my
> own patch ?

Yes. I did so already.

> 2. If I have several patches, that need to be together, how should I
> proceed with pushing them together to gerrit ? (If I want to have
> three patches in the same review ID from gerrit) ?

Yes. They will depend on each other and gerrit will make sure the second does
not get pushed before the first etc.

> [ADMIN]
> 1. I followed the asked process of setting up an account, etc..
> mailed to Norbert so he can match my fd.o account to gerrit. Well,
> how may I now see that I actually have / will have commit access to
> gerrit repo ?

I guess only admins can see that directly from the gerrit UI. But we can
publish the list a week before we make the final switch so everyone can check
himself and will ping those missing directly.

> 2. When my ssh key expires, it is enough to just change it into my
> gerrit account ?

I dont know why that wouldnt be sufficient, I have not tested though ;)
 
> 3. May be a stupid remark, but why do not use this way (opening
> gerrit account ) then to ask for license stuff ? You would have to
> ack to be able to open account, or to load patches. This would ease,
> ensure the process towards this goal ?

Gerrit has already some git hooks that way of making sure a CLA was signed.
Although we dont need a CLA we indeed can use that to get rid of manual
license statements. That would need some research, setup and tweaking, but
might be well worth it in the end. So yeah, if you want to contribute some
research on how those setups work it would be welcome.

Thanks for the feedback!

Best,

Bjoern
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Norbert Thiebaud Norbert Thiebaud
Reply | Threaded
Open this post in threaded view
|

Re: [ANN] Please use Gerrit from now on for Patch Review

In reply to this post by Pierre-André Jacquod
On Thu, Jun 21, 2012 at 12:08 PM, Pierre-André Jacquod
<[hidden email]> wrote:
> I push to gerrit and let tinderbox run. In case of success I get ? a +1 of
> each tinderbox ?
Right now we don;t have tinderbox doing that yet... but when we do,
you'll need to asd a Review +1 to get the auto-build happen

> How do I know it passes with success.
You should get email notification of stuff happening on your patches
(at least you can set that up in your setting)
so you'll get a notification of the activity of the buildbot. depnding
on the sucess/failure, they will but Verify +1 or -1 on your patch

> Once tinderbox is successful, since my change is small or I am confident
> with, I put a +2 to my patch, which means this will insert automatically it
> to master.
No it means that you will  then click 'publish and submit' as you set the +2
(that is once we swtich to gerrit being the main git repo and fdo
being read-only)

> So I will be able to positive review my own patch ?

yes

>
> 2. If I have several patches, that need to be together, how should I proceed
> with pushing them together to gerrit ? (If I want to have three patches in
> the same review ID from gerrit) ?
Yes, gerrit keep track that a patch 'depend on' the one before it.

>
> [ADMIN]
> 1. I followed the asked process of setting up an account, etc.. mailed to
> Norbert so he can match my fd.o account to gerrit. Well, how may I now see
> that I actually have / will have commit access to gerrit repo ?

in your setting/Groups on gerrit you can see the groups you are in

>
> 2. When my ssh key expires, it is enough to just change it into my gerrit
> account ?
yes, although I'm confused with 'expiration' for ssh-key  aren't you
mixing gpg-key and ssh-key concept ?
>
> 3. May be a stupid remark, but why do not use this way (opening gerrit
> account ) then to ask for license stuff ?

There is a hidden/poorly documented feature in gerrit to do just that
(CLA ack) but I haven't experimented with it yet...

> You would have to ack to be able
> to open account, or to load patches. This would ease, ensure the process
> towards this goal ?
apparently there is a hook that get call when CLA is agree upon, which
should allow to automatically promote a user to a different group upon
clearance of the license issue.
This is all doable, but not yet on the top of the to-do list :-)

Norbert
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Michael Stahl-2 Michael Stahl-2
Reply | Threaded
Open this post in threaded view
|

Re: [ANN] Please use Gerrit from now on for Patch Review

In reply to this post by Jan Holesovsky
On 19/06/12 14:44, Jan Holesovsky wrote:

> Hi Norbert,
>
> On 2012-06-19 at 07:23 -0500, Norbert Thiebaud wrote:
>
>>> Thanks Robert for doing that!  Please - is there any chance to use cgit
>>> instead, so that it is compatible / familiar with the freedesktop
>>> browsing?
>>
>> in theory it is... but gitweb was working out of the box....
>> Is there any significant difference ?
>
> Not that significant; I think the familiarity with what we have is the
> greatest plus.
>
> cgit is a bit better usable in the sense that you do not have to click
> that much there as in gitweb, and also looks better (not overwhelming
> you with possibilities you won't use anyway, the color bars to see how
> much has changed in the file, less distracting colors in diffs)
>
> https://gerrit.libreoffice.org/gitweb?p=core.git;a=commitdiff;h=e16ac6937245977b7900c48a408be2c589f6a0a6;hp=951ed65898208a7597c813141ed160716708d348
>
> vs.
>
> http://cgit.freedesktop.org/libreoffice/core/commit/?id=e16ac6937245977b7900c48a408be2c589f6a0a6

the only difference seems to me the diffstat in the cgit page.

as far as i'm concerned they both have the same critical usability flaw:

the title of the page does not contain anything from the commit message,
which means that having 10 commits open as tabs in a single browser
window is a pretty awful experience.


_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Lionel Elie Mamane Lionel Elie Mamane
Reply | Threaded
Open this post in threaded view
|

Re: [libreoffice-projects] [ANN] Please use Gerrit from now on for Patch Review

In reply to this post by Bjoern Michaelsen
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
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Bjoern Michaelsen Bjoern Michaelsen
Reply | Threaded
Open this post in threaded view
|

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
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Lionel Elie Mamane Lionel Elie Mamane
Reply | Threaded
Open this post in threaded view
|

Re: [ANN] Please use Gerrit from now on for Patch Review

In reply to this post by Pierre-André Jacquod
On Thu, Jun 21, 2012 at 07:08:46PM +0200, Pierre-André Jacquod wrote:

> Just to say, I basically just really do not like the fact of having
> to use openId, would prefer to have an account at fd.o. I did it
> really for the LibO, a kind of forced to. And NO, I do NOT have any
> google, flickr, facebook or other account. Do not trust them enough
> to put my data there, so to use them as authentication.... But no
> choice (!) I kind of understood. [+1 for / to Lionel thread]

Would you be made much happier if there would be a TDF or FDO OpenID
provider, which you would use only to authenticate to TDF (or FDO)
services?

--
Lionel
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Lionel Elie Mamane Lionel Elie Mamane
Reply | Threaded
Open this post in threaded view
|

Re: [ANN] Please use Gerrit from now on for Patch Review

In reply to this post by Bjoern Michaelsen
On Mon, Jun 18, 2012 at 12:09:49PM +0200, Bjoern Michaelsen wrote:

> gerrit is documented and ready to go. Please use it for code review
> as much as possible now as it simplifies things a lot over manual
> patch fiddling on mailing lists.

So, I went to https://gerrit.libreoffice.org/#/c/255/

Then I click on "sw/source/core/bastyp/SwSmartTagMgr.cxx" just to
*see* what this patch is about. I go back to previous page, and now
this line has a green mark in "Reviewed". This gives the idea that
I somehow approve of these changes. I DO NOT, I have only glanced at
them and have not emitted any opinion on the changes. Yes, I can
uncheck the "reviewed" checkbox, but frankly this is *very*
dangerous. I think it should *really* *really* *really* mark something
as "reviewed" by me only *after* explicit request by me AND NOT
AUTOMATICALLY because I merely loaded the diff in my browser!!!

When I click "Diff All Side-by-Side" (or "Diff All Unified"), it shows
me only the first file with a link to the (diff) of the next file. I
expected to see the diff for *all* files on one page. That is much
more convenient to check if the changes in one file match the changes
in another file. For example: the signature of a function is called in
foo.h and everywhere it is called (in bar.cxx and qux.cxx), the call
is adapted correctly.

--
Lionel
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Bjoern Michaelsen Bjoern Michaelsen
Reply | Threaded
Open this post in threaded view
|

Re: [ANN] Please use Gerrit from now on for Patch Review

On Mon, Jul 02, 2012 at 06:13:50PM +0200, Lionel Elie Mamane wrote:
> Then I click on "sw/source/core/bastyp/SwSmartTagMgr.cxx" just to
> *see* what this patch is about. I go back to previous page, and now
> this line has a green mark in "Reviewed". This gives the idea that
> I somehow approve of these changes.

No it does not. You approve the changes by giving a +2 code review.

> When I click "Diff All Side-by-Side" (or "Diff All Unified"), it shows
> me only the first file with a link to the (diff) of the next file. I
> expected to see the diff for *all* files on one page. That is much
> more convenient to check if the changes in one file match the changes
> in another file. For example: the signature of a function is called in
> foo.h and everywhere it is called (in bar.cxx and qux.cxx), the call
> is adapted correctly.

There is gitweb to see all changes on one page. We might patch the diff-all
button to link there too (although you cannot do inline comments there).

Best,

Bjoern
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Next » 12345 « Prev