Thread (1 message) 1 message, 1 author, 2023-02-06

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

From: Sergey Organov <hidden>
Date: 2023-02-06 21:36:06

Junio C Hamano [off-list ref] writes:
Sergey Organov [off-list ref] writes:
quoted
Junio C Hamano [off-list ref] writes:

quoted
* 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: [ref]
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help