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

[Sergey Organov <sorganov@gmail.com>]

Hi, Junio!

Sergey Organov <sorganov@gmail.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> 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.

This is still marked as "probably won't merge" in the recent "what's
cooking". Could it be merged, please?

Thanks,
-- Sergey Organov