Re: Setup non-pushing gerrit instance for glibc.

[Joseph Myers <joseph at codesourcery dot com>]

On Fri, 25 Oct 2019, Simon Marchi wrote:

> > 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.
> 
> From my experience, git diff doesn't really get the function name, it just
> gives you the first preceding line that starts with a non-whitespace.  It
> often "breaks" when you have labels or preprocessor directives at column 0,
> for example:

Yes.  It's a reasonable heuristic that works in many common cases (and can 
be configured with a diff attribute in .gitattributes and a corresponding 
diff.<language>.xfuncname setting in .git/config, if desired).  Sometimes 
the default amount of context in a diff isn't enough either, and sometimes 
code is rearranged in a way that makes diff output unhelpful.

I'd like reasonable heuristics to be used in the gerrit messages so that 
what they show is sufficient in most cases for it to be possible to fully 
follow and contribute to the discussion via email without needing to go to 
the website.  Showing the diff hunk, with default settings (so the same 
logic as used by default by git diff to identify a function name) seems 
like a reasonable heuristic for that purpose.

> > 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
> 
> In Gerrit, you'd probably use topics to indicate that many changes have
> a common purpose (whether they are interdependent or not).
> 
> https://gerrit-review.googlesource.com/Documentation/intro-user.html#topics

If using a topic were to mark the patches as a patch series (including 
marking email subjects and threading them accordingly), that would be 
fine.  I don't think it's necessary for simply pushing multiple changes at 
once to be the way you cause something to be handled as a patch series, if 
topics are the idiomatic concept in gerrit that corresponds to a patch 
series as generated by git-format-patch.

> > 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.
> 
> I don't think there would be a problem with that, except that the two emails
> on the mailing would not be threaded together.  More thoughts about that
> below.

Well, there would be issues if you wanted gerrit to push changes itself.  
And how does the "identify this as being the same change" system (for 
knowing when something has been committed) work if multiple separately 
reviewed patches are squashed into one commit?

(Is the valid syntax for Change-Ids documented anywhere?  In particular, 
is it OK to write a human-readable Change-Id oneself?)

-- 
Joseph S. Myers
joseph@codesourcery.com