git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Gal Paikin <paiking@google.com>
Cc: git@vger.kernel.org
Subject: Re: Updating the commit message for reverts
Date: Tue, 24 Dec 2019 11:15:42 -0800	[thread overview]
Message-ID: <xmqq7e2l5ych.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <CAEsQYpNtgMgwjVSOYB9vn-YPvKyKPZ2yZ3NigAVe3PztTN4v8w@mail.gmail.com> (Gal Paikin's message of "Tue, 24 Dec 2019 12:06:51 +0100")

Gal Paikin <paiking@google.com> writes:

> I work on the Gerrit team and I would like to change the default
> behavior for suggested commit messages for Reverts.
> Currently, if the user is trying a change called '"Revert "change X"',
> the suggested commit message would be 'Revert "Revert "Change X""'
> which is silly, since sometimes users want to revert the same change
> many times.
>
> The suggestion is to change the behavior to "Revert^N" instead of
> multiple Reverts one after another.
>
> I'm happy to change those things in Gerrit, but it would also be nice
> if it were changed inside of Git.
>
> What do you think?

I do not _think_ anybody lets the exact phrase 'Revert ...' in the
log message to be read by scripts to perform machannical action, so
in that sense it may be OK to special-case the reversion of a revert
and rephrase it in a more human friendly way.

BUT

 * what does "Revert^47" even mean?  Not just the proposed phrasing
   looks horrible, it is not even clear what happened at the end.
   Was the patch turned out to be OK after all these reversion war,
   or got rejected for now?  It also misleads readers who know Git
   can perform a merge with more than two parents that it may be
   reverting the effect relative to 47th parent of the commit.

   It _might_ be slightly more acceptable to flip the phrase between
   "Revert X" and "Reinstate X" (or "Reapply X"), without saying
   "this is the 47th round of our reversion war".  I dunno.

 * how often does it happen in practice?  If a group of developers
   find themselves reverting and reapplying the same commit more
   than a few times, wouldn't they rather stop and think before
   doing yet another round, which I expect to result in a better fix
   implemented as a separate and brand new patch that takes
   inspiration from a patch that was earlier reverted, and at that
   point it won't be the 47th iteration of reversion war anyway.

So, I am fairly negative on the change in the proposed form as-is.

  reply	other threads:[~2019-12-24 19:16 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-24 11:06 Updating the commit message for reverts Gal Paikin
2019-12-24 19:15 ` Junio C Hamano [this message]
2019-12-27 10:13   ` Gal Paikin
2019-12-28 13:20     ` Danh Doan
2019-12-30 16:52     ` Junio C Hamano
2019-12-30 19:55 ` Jonathan Nieder
2019-12-30 19:59 ` Jonathan Nieder
2019-12-30 20:33   ` Oswald Buddenhagen

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

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

  git send-email \
    --in-reply-to=xmqq7e2l5ych.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=paiking@google.com \
    /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 NNTP newsgroup(s).