Re: [PATCH v3 2/2] doc: revert: add discussion

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Oswald Buddenhagen <oswald.buddenhagen@xxxxxx> 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@xxxxxx> 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).



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux