From: Linus Arver <linusa@google.com>
To: Junio C Hamano <gitster@pobox.com>,
Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Cc: Phillip Wood <phillip.wood123@gmail.com>,
git@vger.kernel.org, Kristoffer Haugsbakk <code@khaugsbakk.name>
Subject: Re: [PATCH v2] sequencer: beautify subject of reverts of reverts
Date: Fri, 28 Jul 2023 10:36:46 -0700 [thread overview]
Message-ID: <owly3518ey4x.fsf@fine.c.googlers.com> (raw)
In-Reply-To: <xmqqpm4c5ax9.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> The usefulness of the message comes from its information content.
> What do we want to read out of these messages? I think we want
> a title that immediately lets us know three things:
>
> (1) What the original patch was about.
> (2) What the final state is.
> (3) How involved was the road to get to the final state has been.
>
> As to (1), we are not proposing to lose what comes "Revert", so this
> information is not lost under any proposal we have seen so far in
> the discussion.
Agreed.
> As to (2), with the current "Revert" -> "Revert Revert" -> "Revert
> Revert Revert" -> ..., you have to count, which is cumbersome and
> does not give you an immediate access to that information. With
> "Revert^n", you'd see if n is even or odd to determine, which is
> much better than the status quo
I actually think "Revert^n" is much worse than what we have, mainly
because the "^n" syntax is already used in other subcommands (e.g.,
rebase). It may well be that we already have overloaded syntax
elsewhere, but we should avoid overloading where possible.
> , but it takes practice to interpret.
> With "Revert" -> "Reapply" -> "Revert Reapply" -> "Reapply Reapply"
> -> ..., the first word would give you the final state immediately.
I agree. But to take the idea further, maybe "Reapply Reapply" should be
shortened to just
Reapply* "original title"
and likewise any ultimate _removal_ of the commit (no matter the exact
pairwise count of the word "Revert") should be shortened to
Revert* "original title"
? The trailing asterisk in each case would indicate that this
reapplication was not the first reapplication. We could then put
in the commit message body text something like
--8<---------------cut here--------------------------------->8---
*NOTE: This is not the first time we've reverted XXX.
--8<---------------cut here----------------^---------^------>8---
| |
| Commit SHA.
|
Could be "reapplied" if
"Reapply*" is the title
prefix.
to help stress point (3) that you described in your list above.
> We want to know (3), because between a change whose revert was
> reverted and a change that hasn't been involved in any revert, there
> may be no difference in the end result, the former is likely to be
> trickier and merits more careful inspection than the latter.
Agreed.
> With
> "Revert^n", we read how large the number n is to find the
> information. With the current "the Revert repeated number of times"
> or your "a pair of frontmost Reverts become one Reapply", the length
> of the Revert/Reapply prefix conveys this information, but this is
> associated with the cost of pushing the original title further to
> the right and hard to read/find.
I agree that trying to keep the title short is very important. I didn't
think too deeply about this point previously, but I think it is equally
important as the other points you've enumerated above.
> Note that, while the number of
> times revert-reapply sequence took place is a useful piece of
> information, the exact number may not be all that important.
Agreed. For this reason I would prefer just using a single asterisk (as
in my example) after the first occurrence of the revert/reapplication.
> And from the above discussion, I wonder if the following would be a
> good place to stop:
>
> - The first revert is as before: Revert "original title"
> - A revert of a revert becomes: Reapply "original title"
> - A revert of a reapply becomes: Revert Reapply "original title"
> - A revert of "Revert Reapply" becomes: Reapply Reapply "original title"
> - A revert of "Reapply Reapply" becomes: Revert Reapply "original title"
>
> In other words, we accept the fact that wedo not need exact number
> of times reversions were done, and use that to simplify the output
> to make sure we will not spend more than two words in the front of
> the title. That would help to keep the original title visible,
> while still allowing us to distinguish the ones that was reverted up
> to four times (and "Revert Reapply" and "Reapply Reapply" only tell
> us "final state is to (discard|accept) the original but it took us
> _many_ times", without saying exactly how many).
I very much like your idea of just "stopping the recursion" early on.
The only difference is that I would prefer to drop the second "Reapply"
word in your examples and replace them with an asterisk (this makes them
the same as in my example).
I think having this second word "Reapply" would make the title
potentially hurt readability, especially because the first word is
really the only one users should be looking at anyway to figure out the
final state. Another reason for dropping it is because a well-written
commit message title should have a single verb near the beginning, but
your style would break this convention.
If you don't like the "asterisk" idea, another form might be
- A revert of a reapply becomes: Revert reapplication of "original title"
- A revert of the above becomes: Reapply revert of "original title"
- A revert of the above becomes: Revert reapplication of "original title"
- A revert of the above becomes: Reapply revert of "original title"
and so forth.
next prev parent reply other threads:[~2023-07-28 17:37 UTC|newest]
Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-28 8:35 [PATCH v2] sequencer: beautify subject of reverts of reverts Oswald Buddenhagen
2023-04-28 18:35 ` Junio C Hamano
2023-04-28 19:11 ` Oswald Buddenhagen
2023-05-01 16:44 ` Junio C Hamano
2023-05-01 19:10 ` Oswald Buddenhagen
2023-05-01 19:12 ` Junio C Hamano
2023-05-05 17:25 ` Junio C Hamano
2023-05-17 9:05 ` Phillip Wood
2023-05-17 10:00 ` Oswald Buddenhagen
2023-05-17 11:20 ` Phillip Wood
2023-05-17 17:02 ` Oswald Buddenhagen
2023-05-18 9:58 ` Phillip Wood
2023-05-18 16:28 ` Junio C Hamano
2023-07-28 5:26 ` Linus Arver
2023-07-28 9:45 ` Oswald Buddenhagen
2023-07-28 15:10 ` Junio C Hamano
2023-07-28 15:37 ` Oswald Buddenhagen
2023-07-28 16:31 ` Junio C Hamano
2023-07-28 16:47 ` Oswald Buddenhagen
2023-07-28 17:36 ` Linus Arver [this message]
2023-08-09 17:15 ` [PATCH v3 1/2] " Oswald Buddenhagen
2023-08-09 17:15 ` [PATCH v3 2/2] doc: revert: add discussion Oswald Buddenhagen
2023-08-10 21:50 ` Linus Arver
2023-08-10 22:00 ` Linus Arver
2023-08-11 15:10 ` Phillip Wood
2023-08-12 6:25 ` Oswald Buddenhagen
2023-08-13 22:09 ` Junio C Hamano
2023-08-14 14:13 ` Oswald Buddenhagen
2023-08-11 12:49 ` Oswald Buddenhagen
2023-08-11 23:00 ` Linus Arver
2023-08-12 7:19 ` Oswald Buddenhagen
2023-09-07 21:29 ` Linus Arver
2023-08-11 15:08 ` Phillip Wood
2023-08-11 17:10 ` Junio C Hamano
2023-08-11 17:05 ` Junio C Hamano
2023-08-11 17:44 ` Re* " Junio C Hamano
2023-08-11 17:53 ` Junio C Hamano
2023-08-11 17:56 ` rsbecker
2023-08-11 18:16 ` Eric Sunshine
2023-08-11 18:16 ` Oswald Buddenhagen
2023-08-11 19:43 ` Junio C Hamano
2023-08-11 15:05 ` [PATCH v3 1/2] sequencer: beautify subject of reverts of reverts Phillip Wood
2023-08-11 16:59 ` Junio C Hamano
2023-08-11 17:13 ` Oswald Buddenhagen
2023-08-21 17:07 ` [PATCH v4 " Oswald Buddenhagen
2023-08-21 17:07 ` [PATCH v4 2/2] git-revert.txt: add discussion Oswald Buddenhagen
2023-08-21 18:32 ` [PATCH v4 1/2] sequencer: beautify subject of reverts of reverts Junio C Hamano
2023-08-23 20:08 ` Taylor Blau
2023-08-23 21:38 ` Junio C Hamano
2023-08-24 6:14 ` Oswald Buddenhagen
2023-09-02 7:20 ` [PATCH v5] " Oswald Buddenhagen
2023-09-02 22:24 ` Junio C Hamano
2023-09-11 20:12 ` Kristoffer Haugsbakk
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=owly3518ey4x.fsf@fine.c.googlers.com \
--to=linusa@google.com \
--cc=code@khaugsbakk.name \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=oswald.buddenhagen@gmx.de \
--cc=phillip.wood123@gmail.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 read-only IMAP folder(s) and NNTP newsgroup(s).