Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: Linus Arver <linusa@google.com>
To: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH v3 2/2] doc: revert: add discussion
Date: Fri, 11 Aug 2023 16:00:53 -0700	[thread overview]
Message-ID: <owly350pfal6.fsf@fine.c.googlers.com> (raw)
In-Reply-To: <ZNYuUh27ByphTH04@ugly>

Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:

> On Thu, Aug 10, 2023 at 02:50:59PM -0700, Linus Arver wrote:
>>Nit: the "doc: revert: add discussion" subject line should probably be more
>>like "revert doc: suggest adding the 'why' behind reverts".
>>
> this is counter to the prevalent "big endian" prefix style, and is in 
> this case really easy to misread.
> i also intentionally kept the subject generic, because the content 
> covers two matters (the reasoning and the subjects, which is also the 
> reason why this is a separate patch to start with).

I think the phrase "add discussion" in "doc: revert: add discussion"
doesn't add much value, because your patch's diff is very easy to read
(in that it adds a new DISCUSSION section). I just wanted to replace it
with something more useful that gives more information than just repeat
(somewhat redundantly) what is obvious by looking at the patch.

I also learned recently that there should just be one colon ":" in the
subject, which is why I suggested "revert doc" as the prefix instead of
"doc: revert: ...".

>>Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:
>>> +DISCUSSION
>>> +----------
>>> +
>>> +While git creates a basic commit message automatically, you really
>>> +should not leave it at that. In particular, it is _strongly_
>>> +recommended to explain why the original commit is being reverted.
>>> +Repeatedly reverting reversions yields increasingly unwieldy
>>> +commit subjects; latest when you arrive at 'Reapply "Reapply
>>> +"<original subject>""' you should get creative.
>>
>>The word "latest" here sounds odd. Ditto for "get creative".
>>
> yeah, i suppose. i wasn't sure how formal i should make it - things 
> aren't consistent to start with.

For our discussion I will define "formal" style as the writing style
used in traditional reference texts, for example dictionaries and
encyclopedias.

For manpages, I think we should stick to formal style as much as
possible. The main concern I have is for readers of our manpages where
English may not be their first language. Although I understood what you
meant by the phrase "get creative", others may not understand so
readily. If there are places in our manpages where we do not use formal
style, I think we should fix them.

For other types of documentation like tutorials, I think the style
doesn't have to be as formal, (in fact it should be as informal as
possible) because a tutorial should be enjoyable to read from beginning
to end. This is unlike a manpage where most of the time the user reads
specific (sub)sections to get the exact, precise information they need
(just like looking up a word in a dictionary).

>> How about the following rewording?
>>
>>    While git creates a basic commit message automatically, it is
>>    _strongly_ recommended to explain why the original commit is being
>>    reverted. In addition, repeatedly reverting the same commit will
>>    result in increasingly unwieldy subject lines,
>
>>for example 'Reapply "Reapply "<original subject>""'.
>>
> you turned it from a suggested threshold into an example. at this point 
> it appears superfluous to me.
>
>>Please consider rewording such
>>    subject lines to reflect the reason why the original commit is being
>>    reapplied again.
>>
> the reasoning most likely wouldn't fit into the subject.

Hence the language "to _reflect_ the reason", because the "reason"
should belong in the commit message body text.

> also, the original request to explain the reasoning applies 
> transitively, so i don't think it's really necessary to point it out 
> explicitly.

It may be that a user will think only giving the revert reason in the
body text is enough, while leaving the subject line as is. I wanted to
break this line of thinking by providing additional instructions.

> On Thu, Aug 10, 2023 at 03:00:41PM -0700, Linus Arver wrote:
>>Hmph, "repeatedly reverting the same commit" sounds wrong because
>>strictly speaking there is only 1 "same commit" (the original commit).
>>Perhaps
>>
>>    In addition, repeatedly reverting the same progression of reverts will
>>
>>or even
>>
>>    In addition, repeatedly reverting the same revert chain will
>>
>>is better here?
>>
> we used "recursive reverts" elsewhere. but i'm not sure whether that's 
> sufficiently intuitive and formally correct.

Agreed. We might have to just define the correct phrasing ourselves in
"man gitglossary". Surprisingly I see that the term "revert" (which can
be both a verb _and_ a noun) is not defined there.

> [...]
>
> so in summary, how about:
>
>      While git creates a basic commit message automatically, it is
>      _strongly_ recommended to explain why the original commit is being
>      reverted. In addition, repeatedly reverting reversions will
>      result in increasingly unwieldy subject lines. Please consider 
>      rewording these into something shorter and more unique.

This is definitely better. But others in this thread have already
commented that my version looks good (after seeing your version also,
presumably).

  reply	other threads:[~2023-08-11 23:03 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
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 [this message]
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=owly350pfal6.fsf@fine.c.googlers.com \
    --to=linusa@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=oswald.buddenhagen@gmx.de \
    /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).