All of lore.kernel.org
 help / color / mirror / Atom feed
From: Glen Choo <chooglen@google.com>
To: Sergey Organov <sorganov@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: so/diff-merges-more (was Re: What's cooking in git.git (Feb 2023, #01; Thu, 2))
Date: Fri, 10 Mar 2023 13:19:06 -0800	[thread overview]
Message-ID: <kl6la60kqp85.fsf@chooglen-macbookpro.roam.corp.google.com> (raw)
In-Reply-To: <878rg5vgvc.fsf@osv.gnss.ru>

Sergey Organov <sorganov@gmail.com> writes:

> Glen Choo <chooglen@google.com> writes:
>
>> Sergey Organov <sorganov@gmail.com> writes:
>>
>>> I already agreed long ago that to be on the safe side we shouldn't
>>> simply change -m nowadays, and addressed that concern by putting -m
>>> behavior change under configuration option in the current series. So
>>> what's the reason of rejection?
>>
>> Before we start talking in circles again, I think the responses
>> elsewhere in the thread still accurately capture mine and Junio's
>> views, e.g.:
>>
>>   https://lore.kernel.org/git/xmqqedr28wwb.fsf@gitster.g
>> https://lore.kernel.org/git/kl6lr0v2i0gn.fsf@chooglen-macbookpro.roam.corp.google.com/
>
> There were no any response from you to my recent very direct question
> (that Junio did somewhat address), but what I figured from your and
> Junio explanation so far looks like:

Yes, thank you for summarizing the discussion. I think the following
sums up my position quite accurately, and I found this very helpful.

> 1. The fact that -m does not imply -p is a mistake. There is no any
>    reasons this exact behavior could be useful. As such, it does not
>    make sense to support this exact behavior in --diff-merges. So the
>    reject of --diff-merges=[no-]hide.

Yes.

> 2. This mistake is too dangerous to fix due to subtle compatibility
>    problems, so we can't just fix -m behavior. Thus the reject of my
>    earlier patch "let -m imply -p".

Yes, unfortunately. If we didn't have compatibility problems, "-m imply
-p" would be very good IMO. I am not that hopeful, but perhaps there are
ways to get users to move off this compatibility problem. It would be
something we would have to learn as a project though, it is not a
problem unique to this series.

> 3. Moving behavior change under option is not worth it, as nobody
>    presumably needs this fixed -m behavior anyway (at least among 2
>    persons that are actually opposing the changes). So the reject
>    of "add diffMerges-m-imply-p configuration option" patch.

Yes, given how specific the option is (I think it would be the only
config option that exists to change the behavior of a CLI option) I
don't think the option is worth it in light of alternatives like a new
"-d" flag or supporting "-mp".

> 4. Staring in the face inconsistency between -m and the rest
>    of short diff-merge options is not significant enough to reconsider
>    any of the above rejects.

Yes. Git carries many such inconsistencies.

> Did I outline your positions correctly and to the full extent? Anything
> else I forgot?

I cannot think of anything else. Thank you.

> I want to make sure I got it exactly right, as I'd still like to tweak
> my proposals, and I'd like to avoid the troubles I got into as much as
> possible.

Thanks. I hope that there is a good way forward for your proposals.

>
> Thanks,
> -- Sergey Organov

  reply	other threads:[~2023-03-10 21:19 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-03  4:02 What's cooking in git.git (Feb 2023, #01; Thu, 2) Junio C Hamano
2023-02-04  9:33 ` Sergey Organov
2023-02-06 18:35   ` Junio C Hamano
2023-02-06 21:35     ` Sergey Organov
2023-03-01 18:40       ` Sergey Organov
2023-03-01 22:15         ` Junio C Hamano
2023-03-01 22:26           ` Sergey Organov
2023-03-01 23:54             ` Glen Choo
2023-03-02 14:38               ` Sergey Organov
2023-02-07  4:06   ` so/diff-merges-more (was Re: What's cooking in git.git (Feb 2023, #01; Thu, 2)) Glen Choo
2023-02-07 12:50     ` Sergey Organov
2023-03-02  0:37       ` Glen Choo
2023-03-02 16:15         ` Junio C Hamano
2023-03-02 16:57           ` Sergey Organov
2023-03-06 22:22             ` Glen Choo
2023-03-07 10:02               ` Sergey Organov
2023-03-07 17:54                 ` Junio C Hamano
2023-03-08 22:19                   ` Sergey Organov
2023-03-08 23:08                     ` Junio C Hamano
2023-03-09 13:54                       ` Sergey Organov
2023-03-09 17:43                         ` Glen Choo
2023-03-09 19:56                           ` Sergey Organov
2023-03-10 21:19                             ` Glen Choo [this message]
2023-03-10 21:47                             ` Junio C Hamano
2023-03-17 14:18                               ` Sergey Organov
2023-03-18  0:08                                 ` Junio C Hamano
2023-03-25 16:55                                   ` Sergey Organov
2023-03-29  7:43                                     ` Sergey Organov
2023-03-29  8:06                                     ` Sergey Organov
2023-02-08 17:22 ` ds/bundle-uri-5 (was: " Victoria Dye

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=kl6la60kqp85.fsf@chooglen-macbookpro.roam.corp.google.com \
    --to=chooglen@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=sorganov@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.