Re: [PATCH 0/5] diff-merges: more features

From: Glen Choo <chooglen@google.com>
Date: Dec 8, 2022, 23:05 UTC
Message-ID: <kl6lcz8tebtk.fsf@chooglen-macbookpro.roam.corp.google.com>
In-Reply-To: <87tu26arzy.fsf@osv.gnss.ru>

Sergey Organov <sorganov@gmail.com> writes:

Show 11 quoted lines

> The last time '-m' issue appeared on the list, it all started here:
>
> https://lore.kernel.org/git/CAMMLpeR-W35Qq6a343ifrxJ=mwBc_VcXZtVrBYDpJTySNBroFw@mail.gmail.com/
>
> In particular, the final patch and its revert is deeper down this tread:
>
> https://lore.kernel.org/git/20210520214703.27323-11-sorganov@gmail.com/#t
>
> and
>
> https://lore.kernel.org/git/YQyUM2uZdFBX8G0r@google.com/

Thanks, these provide extremely helpful context :) In particular:

- Junio describes this "do nothing unless -p" is given behavior as an
  accident [1].
- Jonathan Nieder notes that this change accidentally broke scripts
  where "-m" probably wasn't doing anything useful, but we wanted to
  avoid breaking the scripts for backwards compatibility anyway [2].

I got the sense that the closest thing to an intentional use case of "-m" is for users who thought that "-m" would affect path limiting [3], although it doesn't actually do that. So what I've reads so far suggests that "do nothing unless -p" (aka --diff-merges=hide) is not actually useful, and we should just drop it.

We could keep the warning for "-m" without "-p" (Patch 5), and recommend "--diff-merges=(on|m)".

[1] https://lore.kernel.org/git/xmqqwnsl93m3.fsf@gitster.g/ [2] https://lore.kernel.org/git/YQtYEftByY8cNMml@google.com/ [3] https://lore.kernel.org/git/YQyUM2uZdFBX8G0r@google.com/

Show 5 quoted lines

>
> Where do you prefer these references to be put, in the cover letter, in
> the commit message, or in both places?
>
> -- Sergey Organov