git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Duy Nguyen" <pclouds@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Git Mailing List" <git@vger.kernel.org>,
	"Phillip Wood" <phillip.wood@dunelm.org.uk>,
	andals@crustytoothpaste.net,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH/RFC] Support --append-trailer in cherry-pick and revert
Date: Wed, 7 Nov 2018 20:29:01 -0500	[thread overview]
Message-ID: <20181108012901.GB10148@sigill.intra.peff.net> (raw)
In-Reply-To: <xmqqh8gs9zh3.fsf@gitster-ct.c.googlers.com>

On Thu, Nov 08, 2018 at 09:36:56AM +0900, Junio C Hamano wrote:

> Duy Nguyen <pclouds@gmail.com> writes:
> 
> > There is still one thing to settle. "revert -m1" could produce
> > something like this
> >
> >     This reverts commit <SHA1>, reversing
> >     changes made to <SHA2>.
> 
> I do not think it is relevant, with or without multiple parents, to
> even attempt to read this message.
> 
> The description is not meant to be machine readable/parseable, but
> is meant to be updated to describe the reason why the reversion was
> made for human readers.  Spending any cycle to attempt interpreting
> it by machines will give a wrong signal to encourage people not to
> touch it.  Instead we should actively encourage people to take that
> as the beginning of their description.
> 
> I even suspect that an update to that message to read something like
> these
> 
> 	"This reverts commit <SHA-1> because FILL IN THE REASONS HERE"
> 
> 	"This reverts commit <SHA-1>, reversing changes made to
> 	 <SHA-1>, because FILL IN THE REASONS HERE"

Yeah, that was the point I was trying to make earlier today in this
thread. I really think of this as a human-readable message.

But as far as how this impacts the addition of a trailer: once we have a
machine-readable "Reverts:", people naturally may want to know about
"Reverts" for older commits. Our options are:

  1. say "sorry, there was no machine-readable version back then"

  2. try to parse our "This reverts" message and normalize it into
     "Reverts" for their benefit.

Before introducing the new footer, it's worth thinking about the
end-game here. If we do (1), then people may want to parse themselves.
They are stuck parsing both the old and new (to handle old and new
commits). We could make life a little easier on them if we included
_both_ the English text and the machine-readable version. And then if
they just chose to parse the English, it would work for old and new.

But I guess that is really just a losing battle, if we consider the
English to be changeable. And doing it ourselves in (2) is really just
pushing that losing battle onto ourselves.

So if we are comfortable with saying that this is a new feature to have
the machine-readable trailer version, and there isn't a robust way to
get historical revert information (because there really isn't[1]), then
I think we can just punt on any kind of trailer-normalization magic.

-Peff

[1] Thinking back on reverts I have done, they are often _not_
    straight-up reverts. For example, I may end up dropping half of a
    commit, but leaving some traces of it in place in order to build up
    the correct solution on top (i.e., fixing whatever problem caused me
    to revert in the first place). I list those as "this is morally a
    revert of 1234abcd...", which is definitely not machine readable. ;)

  reply	other threads:[~2018-11-08  1:29 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-04  7:22 [PATCH/RFC] sequencer.c: record revert/cherry-pick commit with trailer lines Nguyễn Thái Ngọc Duy
2018-11-04 16:45 ` Phillip Wood
2018-11-04 17:41   ` Duy Nguyen
2018-11-04 21:12     ` Phillip Wood
2018-11-05 21:57     ` Johannes Schindelin
2018-11-04 18:10 ` [PATCH/RFC v2] " Nguyễn Thái Ngọc Duy
2018-11-04 21:30   ` brian m. carlson
2018-11-05 16:08     ` Duy Nguyen
2018-11-06 17:16   ` [PATCH/RFC] Support --append-trailer in cherry-pick and revert Nguyễn Thái Ngọc Duy
2018-11-06 17:48     ` Ævar Arnfjörð Bjarmason
2018-11-06 22:11       ` Jeff King
2018-11-06 22:29         ` Jeff King
2018-11-07  0:42         ` Junio C Hamano
2018-11-07 15:30         ` Duy Nguyen
2018-11-07 21:02           ` Jeff King
2018-11-08  0:36           ` Junio C Hamano
2018-11-08  1:29             ` Jeff King [this message]
2018-11-08  3:34               ` Junio C Hamano
2018-11-07  0:31     ` Junio C Hamano
2018-11-05  0:56 ` [PATCH/RFC] sequencer.c: record revert/cherry-pick commit with trailer lines Junio C Hamano
2018-11-05 16:17   ` Duy Nguyen
2018-11-06  1:44     ` Junio C Hamano
2018-11-06  8:57 ` Ævar Arnfjörð Bjarmason
2018-11-06 16:13   ` Duy Nguyen
2018-11-06 17:44     ` Ævar Arnfjörð Bjarmason

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: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20181108012901.GB10148@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=andals@crustytoothpaste.net \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    --cc=phillip.wood@dunelm.org.uk \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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