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 14:48:48 +0000	[thread overview]
Message-ID: <alpine.DEB.2.21.1910251438220.20732@digraph.polyomino.org.uk> (raw)
In-Reply-To: <abf81e1a-3920-ef34-62b2-a582cbdc5f1c@polymtl.ca>

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

  parent reply	other threads:[~2019-10-25 14:49 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 [this message]
2019-10-25 15:48           ` Simon Marchi
2019-10-25 16:10             ` Joseph Myers
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.1910251438220.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).