git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Glen Choo <chooglen@google.com>
Cc: Sergey Organov <sorganov@gmail.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: Thu, 02 Mar 2023 08:15:28 -0800	[thread overview]
Message-ID: <xmqqr0u7ku3j.fsf@gitster.g> (raw)
In-Reply-To: <kl6lh6v43s4g.fsf@chooglen-macbookpro.roam.corp.google.com> (Glen Choo's message of "Wed, 01 Mar 2023 16:37:51 -0800")

Glen Choo <chooglen@google.com> writes:

>> Finally, event without "log.diffMerges-m-imply-p", the rest of the
>> series still makes sense, so if the conclusion is to remove it, let's
>> still merge the rest, please! Let me know, and I'll then remove the
>> patch and will change documentation accordingly.
>
> Oops. Sorry, I missed this the first time I read this message. This
> alone should have warranted a response.

Hmph.  I agreed with you that the last step to add the configuration
would not make sense unless we are planning to break users later,
but I have been under the impression that the remainder were OK
clean-ups.  Perhaps it is mostly because I read them so long ago and
forgot the details X-<.

> I'm not convinced that the series makes sense without
> "log.diffMerges-m-imply-p":
>
> * The main patch is
>
>     diff-merges: implement [no-]hide option and log.diffMergesHide config
>
>   which gives us a way to redefine "-m" as "--diff-merges=hide
>   --diff-merges=on". However, we haven't seen any compelling reasons to
>   use --diff-merges=hide [1]. I'm also fairly convinced that if we go
>   back in time, "-m" wouldn't have the semantics of 'do nothing unless
>   -p is also given', and I don't think we should immortalize a mistake
>   by giving it an explicit option.
>
>   All the other patches in their current form are dependent on this
>   patch going in.
>
> * diff-merges: support list of values for --diff-merges
>
>   This only makes sense if we want to accept multiple values, which we
>   don't without the main patch.

Now you mention it (and show example in the previous bullet point),
I have to agree that we would not want this, not because we do not
want to have --diff-merges with multiple values, but because it
introduces an odd-man-out option that is not "last one wins" that is
not used anywhere else in the UI.

No response does not necessarily mean an agreement, but it indeed
helped in this case to be explicit.

Thanks for the clarification.  

  reply	other threads:[~2023-03-02 16:15 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 [this message]
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
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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=xmqqr0u7ku3j.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=chooglen@google.com \
    --cc=git@vger.kernel.org \
    --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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).