This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Setup non-pushing gerrit instance for glibc.


On Thu, 24 Oct 2019, Simon Marchi wrote:

> 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:

I think the hunk - not just the new version of the code on its own - is 
critical information if someone is commenting on a particular part of a 
change, needed for such comments to be properly readable in context.

> 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.

In that case it should at least show a reasonable amount of context, with 
some indication of what function it is in, like in the diff hunk header.

> 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.

There is a difference of *intent* between changes that depend on one 
another and a patch series.  A patch series is saying:

* there is a common purpose motivating the patches; and

* some patches may best be understood by reference to ones later in the 
series (if e.g. one patch adds an interface that a later one adds users 
for), so the link to those later patches is important for review purposes.

And so a patch series should be sent out to the list for review as such.  
There may be comments on the series as a whole, or on individual patches 
in it.

There's also the variant of patch series that are explained in the cover 
letter (or in other text not intended for the final commit) as being split 
only for review purposes and intended to be squashed for commit, if the 
pieces most convenient for review don't form bisectable commits.  I'm not 
sure how much any review system needs to know about the distinction 
between the two kinds of patch series if it's not pushing the commits 
itself.

> 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.

All of this is perfectly meaningful for patch series - you can do [PATCHv2 
3/3] and then potentially [PATCH 4/3] for D if it's intended as part of 
the series (or just a separate submission for D if it's not intended as 
part of the series).  You can revise patch 1/3 with or without refreshing 
the whole series.

-- 
Joseph S. Myers
joseph@codesourcery.com


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]