From: Phillip Wood <phillip.wood123@gmail.com>
Date: Wed, 17 May 2023 11:20:03 GMT
Subject: Re: [PATCH v2] sequencer: beautify subject of reverts of reverts
Message-ID: <2d416834-ef3e-01a2-6be0-9e88bc0de25e@gmail.com>
In-Reply-To: <ZGSlqAPwaLhgWm6v@ugly>

On 17/05/2023 11:00, Oswald Buddenhagen wrote:
> On Wed, May 17, 2023 at 10:05:51AM +0100, Phillip Wood wrote:
>> On 28/04/2023 09:35, Oswald Buddenhagen wrote:
>>> +        } else if (skip_prefix(msg.subject, "Revert \"", 
>>> &orig_subject)) {
>>> +            if (skip_prefix(orig_subject, "Revert \"", 
>>> &orig_subject)) {
>>
>> I think it is probably worth adding
>>
>>     if (starts_with(orig_subject, "Revert \""))
>>         strbuf_addstr(&msgbuf, "Revert \"");
>>     else
>>
>> here to make sure that we don't end up with a subject starting "Revert 
>> \"Reapply \"Revert ...".
>>
> i can't follow you.
> 
> how is the concern not covered by the subsequent comment?

That comment says that reverting a commit with a subject line

	Revert "Revert some subject"

will result in the new commit having a subject

	Revert "Reapply some subject"

I'm saying that reverting a commit with a subject line

	Revert "Revert "Revert some subject""

should result in the new commit having the subject

	Revert "Revert "Revert "Revert some subject"""

(i.e. at that point we stop trying to be clever) rather than

	Revert "Reapply "Revert some subject""

which I think is what this patch produces.

Best Wishes

Phillip

>>> +                /*
>>> +                 * This prevents the generation of somewhat 
>>> unintuitive (even if
>>> +                 * not incorrect) 'Reapply "Revert "' titles from 
>>> legacy double
>>> +                 * reverts. Fixing up deeper recursions is left to 
>>> the user.
>>> +                 */
> 
> regards,
> ossi