Hi, Junio! Sergey Organov <sorganov@xxxxxxxxx> writes: > Junio C Hamano <gitster@xxxxxxxxx> writes: > >> Sergey Organov <sorganov@xxxxxxxxx> writes: >> >>> Junio C Hamano <gitster@xxxxxxxxx> 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@xxxxxxxxx> >>> >>> 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
