unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Joseph Myers <joseph@codesourcery.com>
To: Simon Marchi <simon.marchi@polymtl.ca>
Cc: Sergio Durigan Junior <sergiodj@redhat.com>,
	Carlos O'Donell <carlos@redhat.com>,
	libc-alpha <libc-alpha@sourceware.org>,
	Jonathan Nieder <jrnieder@gmail.com>
Subject: Re: Setup non-pushing gerrit instance for glibc.
Date: Fri, 25 Oct 2019 16:10:27 +0000	[thread overview]
Message-ID: <alpine.DEB.2.21.1910251558190.20732@digraph.polyomino.org.uk> (raw)
In-Reply-To: <0374dde9-0b49-0af4-c718-e894b59e680f@polymtl.ca>

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

  reply	other threads:[~2019-10-25 16:10 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-24 19:39 Setup non-pushing gerrit instance for glibc Carlos O'Donell
2019-10-24 21:41 ` Joseph Myers
2019-10-25  0:43   ` Ian Kent
2019-10-25  1:16     ` Jonathan Nieder
2019-10-25 10:21       ` Ian Kent
2019-10-25  1:02   ` Carlos O'Donell
2019-10-25  2:04     ` Sergio Durigan Junior
2019-10-25  3:14       ` Simon Marchi
2019-10-25  4:10         ` Simon Marchi
2019-10-25 14:48         ` Joseph Myers
2019-10-25 15:48           ` Simon Marchi
2019-10-25 16:10             ` Joseph Myers [this message]
2019-10-25 16:28               ` Simon Marchi
2019-10-25  1:25   ` Jonathan Nieder
2019-10-25 14:09 ` Florian Weimer
2019-10-25 15:18 ` Joseph Myers
2019-10-25 17:00   ` Sergio Durigan Junior
2019-10-25 20:33     ` Joseph Myers
2019-10-25 21:06       ` Sergio Durigan Junior
2019-10-25 21:13         ` Joseph Myers
2019-10-27 22:12           ` Sergio Durigan Junior
2019-10-28 17:58             ` Joseph Myers
2019-10-28 19:17               ` Jonathan Nieder
2019-10-28 22:59                 ` Joseph Myers
2019-10-29  1:21                   ` Sergio Durigan Junior
2019-10-25 22:03       ` Jonathan Nieder
2019-10-26 13:53         ` Carlos O'Donell
2019-10-30 18:25 ` Szabolcs Nagy
2019-11-12 18:53 ` Gabriel F. T. Gomes
2019-11-12 19:21   ` Carlos O'Donell
2019-11-12 19:59     ` Florian Weimer
2019-11-12 20:02       ` Jonathan Nieder
2019-11-12 21:10     ` Gabriel F. T. Gomes
2019-11-12 21:13       ` Carlos O'Donell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/libc/involved.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.21.1910251558190.20732@digraph.polyomino.org.uk \
    --to=joseph@codesourcery.com \
    --cc=carlos@redhat.com \
    --cc=jrnieder@gmail.com \
    --cc=libc-alpha@sourceware.org \
    --cc=sergiodj@redhat.com \
    --cc=simon.marchi@polymtl.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).