Re: What's cooking in git.git (Feb 2023, #01; Thu, 2)

From: Sergey Organov <sorganov@gmail.com>
Date: Feb 6, 2023, 21:35 UTC
Message-ID: <87357ischs.fsf@osv.gnss.ru>
In-Reply-To: <xmqqedr28wwb.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

Show 39 quoted lines

> Sergey Organov <sorganov@gmail.com> writes:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>
>>> * so/diff-merges-more (2022-12-18) 5 commits
>>>  - diff-merges: improve --diff-merges documentation
>>>  - diff-merges: issue warning on lone '-m' option
>>>  - diff-merges: support list of values for --diff-merges
>>>  - diff-merges: implement log.diffMerges-m-imply-p config
>>>  - diff-merges: implement [no-]hide option and log.diffMergesHide config
>>>
>>>  Assorted updates to "--diff-merges=X" option.
>>>
>>>  May want to discard.  Breaking compatibility does not seem worth it.
>>>  source: <20221217132955.108542-1-sorganov@gmail.com>
>>
>> Hi Junio,
>>
>> This does not break any compatibility, as far as me and I believe
>> reviewers of these series are aware.
>
> The last paragraphs in the review two months ago still describe what
> this series does fairly accurately, I think.
>
>     These patches do look like a good approach to solve the first point
>     among the "two problems" in the previous round. Thanks for working
>     on it.
>
>     IIRC, the previous round (why is this round marked as v1, by the
>     way?) was reviewed by some folks, so lets wait to hear from them
>     how this round does better.
>
>     Unfortunately, I do not think of any "solution" that would avoid
>     breaking folks, if its end goal is to flip the default, either by
>     hardcoding or with a configuration variable.  IOW, the other one
>     among the "two problems" in the previous round sounds unsolvable.
>     We should question if it was really an "issue" worth "resolving",
>     though.

Well, we may end up flipping or not flipping the default (even though I'd prefer we indeed do), the series are still valid either way, as they allow *me* (or anybody else who prefers more useful '-m' behavior) to flip the switch for myself, locally.

Also, the only patch that got some resistance from reviewers has been removed from the series, so I don't see anything left that'd prevent this from being merged.

From my POV the only remotely questionable patch is:

- diff-merges: issue warning on lone '-m' option

and I hereby agree to remove it if it feels wrong to you.

Let me state it cleanly: once these are accepted, I'll turn log.diffMerges-m-imply-p on for myself, and will suggest it to others who already asked about '-m' inconsistency, or will ask in the future.

Thanks, -- Sergey Organov