Re: Setup non-pushing gerrit instance for glibc.

[Simon Marchi <simon dot marchi at polymtl dot ca>]

Hi all,

Thanks a lot for you comments, it gives a good idea of what the main
pain points are.

On 2019-10-24 10:04 p.m., Sergio Durigan Junior wrote:
> Simon was working on this, and he told me he has a prototype fix that he
> was planning to deploy on our instance.  I don't know the current
> status, though.  I added him to the Cc list.
> 
> BTW, I agree this is a problem that should be fixed; quoting just one
> line from the change is really not sufficient.

This is a prototype I did after spending maybe 15 minutes with the source of
Gerrit, so don't expect too much :).  I was able to tell Gerrit to include
a fixed amount of lines above the line the comment is attached to.

We'll maybe manage to do something a bit smarter, but I don't think we'll
easily be able to quote the _hunk_.  This is where the mail is generated:

https://gerrit.googlesource.com/gerrit/+/refs/heads/master/java/com/google/gerrit/server/mail/send/CommentSender.java#313

That code has access to the lines of the file where the comment was applied.
So if you put your comment on the left pane, in the web UI (e.g. on the base),
then we'll quote the lines of that version.  If you put your comment on right
(e.g. on v1), then we'll quote the lines of that version.  But I don't think
we'll be able to make up a hunk with +'s and -'s.

Also, remember that you can go put a comment on an unchanged section on the
file (e.g. to say "you forgot to update this call"), so there would be no
diff hunk to show for that comment anyway.

I filed this, if you want to subscribe: https://bugs.chromium.org/p/gerrit/issues/detail?id=11804

> The users can configure line wrapping under Settings -> Edit
> Preferences.  This is a per-user configuration; gerrit doesn't have a
> global setting that forces line wrapping.  However, I think it's
> possible to edit the All-Users repository and make this setting
> automatically enabled for everybody.
> 
> /me does some tests...
> 
> Hm, it seems the line wrapping setting doesn't really apply to messages?
> I tried writing a comment without line wrapping here:
> 
>   https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/161/3#message-9f0b67bbd0ef0765d07f417a15f50aa79a5414d5
> 
> And it generated this message:
> 
>   https://sourceware.org/ml/gdb-patches/2019-10/msg00905.html
> 
> I mean, gnus wraps the line for me, but I understand this is my MUA's
> setting, and not something we can really expect.

Yeah, I don't think a user setting can affect that, since there's only one
notification email sent to the list for everybody.

I presume that's something we can control in the generation of the email.

I filed: https://bugs.chromium.org/p/gerrit/issues/detail?id=11805

>>> 5. It would also be useful to have documentation for how someone should 
>>> make a patch series appear appropriately in gerrit if they want to propose 
>>> a series that way.  That means the emails for a patch series should 
>>> include 1/N, 2/N etc. in their subjects (with a 0/N cover letter as 
>>> appropriate).

Putting the cover letter problem aside for now, just addressing the 1/N to N/N
numbering problem.  I think the most honest answer here is: don't expect that so
soon, because it just not in Gerrit's model.

When sending patches by emails, we need to be able to send many patches in a way
that enforces a certain order and shows dependencies between patches.  If you just
sent 10 individual patches instead of a series of 10 patches it would be
impossible to know in what order I need to apply them, or even that they
are related.  So that's why we patch series make sense.

With Gerrit, you push a chain of git commit:

  A <- B <- C

They are naturally related due to them being git commits and having a parent-child
relationship.  If I check out C, I'll automatically check out its parent B and C.
Gerrit doesn't see that as a series, just as individual changes that are dependent
on one another.

Let's say I push a new version of C.  A and B are still at their v1, while C is at
its v2.  Then, if I push D on top of that, D will be at its v1.

Then, I (or even you!) could make a new change E on top of A, where E would be
unrelated to B, C and D (other than sharing A in their ancestry).

I can then decide to rebase A by itself (because master has moved on), which will
create a v2 of A.  All the other changes will still be based on the v1 of A.  I
will be able to rebase the other changes later on the latest version of A, if I
want to.

Since Gerrit allows this kind of non-linear work, where it does not treat chains
of commits as rigid/atomic sequences, I have a hard time coming up with a way how
Gerrit should group that into series as we know them from emails.

Also, I know not everybody here is a fan of web UIs, but browsing the changes
on the web UI lets you see the relation chain between changes (when looking
at a particular change):

  https://nova.polymtl.ca/~simark/ss/ad123r4v.png

So that's how I would see the big picture of how those changes are linked together.

Simon