git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: Eric Sunshine <sunshine@sunshineco.com>,
	 Dragan Simic <dsimic@manjaro.org>,
	 James Liu <james@jamesliu.io>,
	 git@vger.kernel.org
Subject: Re* Re* [PATCH v4 0/3] advice: add "all" option to disable all hints
Date: Mon, 06 May 2024 09:40:31 -0700	[thread overview]
Message-ID: <xmqqbk5i3ncw.fsf_-_@gitster.g> (raw)
In-Reply-To: <34d77e4d-6edb-99d0-7fc5-fea5224654c7@gmx.de> (Johannes Schindelin's message of "Sun, 5 May 2024 13:03:35 +0200 (CEST)")

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> That difference in certainty is the entire reason why I contend that
> `range-diff` and `format-patch --range-diff` need different defaults for
> the creation factor.
>
>> > A similar thread was raised more recently:
>> >
>> >  https://lore.kernel.org/git/rq6919s9-qspp-rn6o-n704-r0400q10747r@tzk.qr/
>>
>> I think I missed this thread.
>
> Heh. I had forgotten about it.

So what's the conclusion of this discussion (have we reached one
yet)?

to me it sounds like everybody is on board to raise the default used
for format-patch.  In the old thread I said "I use something
unreasonably high like 9999", but my "unreasonably high" number
these days is 999, which was used in the earlier weatherbaloon
patch.  The old thread ended with an academic "we know different
defaults are appropriate, but what is the right number then?" but
anything higher (like 999 or even 9999) is better than the default
for range-diff which is 60, I would think.

Here is the patch, this time with a bit of documentation stolen from
the old weather-balloon by Ævar.

----- >8 --------- >8 --------- >8 --------- >8 -----
Subject: [PATCH] format-patch: run range-diff with larger creation-factor

We see too often that a range-diff added to format-patch output
shows too many "unmatched" patches.  This is because the default
value for creation-factor is set to a relatively low value.

It may be justified for other uses (like you have a yet-to-be-sent
new iteration of your series, and compare it against the 'seen'
branch that has an older iteration, probably with the '--left-only'
option, to pick out only your patches while ignoring the others) of
"range-diff" command, but when the command is run as part of the
format-patch, the user _knows_ and expects that the patches in the
old and the new iterations roughly correspond to each other, so we
can and should use a much higher default.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-format-patch.txt | 5 +++++
 builtin/log.c                      | 2 +-
 range-diff.h                       | 6 ++++++
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 728bb3821c..b72f87b114 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -346,6 +346,11 @@ material (this may change in the future).
 	between the previous and current series of patches by adjusting the
 	creation/deletion cost fudge factor. See linkgit:git-range-diff[1])
 	for details.
++
+Defaults to 999 (the linkgit:git-range-diff[1] uses 60), as the use
+case is to show comparison with an older iteration of the same
+topic and the tool should find more correspondence between the two
+sets of patches.
 
 --notes[=<ref>]::
 --no-notes::
diff --git a/builtin/log.c b/builtin/log.c
index c0a8bb95e9..73608ffef9 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -2274,7 +2274,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	}
 
 	if (creation_factor < 0)
-		creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;
+		creation_factor = CREATION_FACTOR_FOR_THE_SAME_SERIES;
 	else if (!rdiff_prev)
 		die(_("the option '%s' requires '%s'"), "--creation-factor", "--range-diff");
 
diff --git a/range-diff.h b/range-diff.h
index 04ffe217be..2f69f6a434 100644
--- a/range-diff.h
+++ b/range-diff.h
@@ -6,6 +6,12 @@
 
 #define RANGE_DIFF_CREATION_FACTOR_DEFAULT 60
 
+/*
+ * A much higher value than the default, when we KNOW we are comparing
+ * the same series (e.g., used when format-patch calls range-diff).
+ */
+#define CREATION_FACTOR_FOR_THE_SAME_SERIES 999
+
 struct range_diff_options {
 	int creation_factor;
 	unsigned dual_color:1;
-- 
2.45.0-31-gd4cc1ec35f


  reply	other threads:[~2024-05-06 16:40 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-24  3:58 [PATCH 0/2] advice: add "all" option to disable all hints James Liu
2024-04-24  3:58 ` [PATCH 1/2] advice: allow advice type to be provided in tests James Liu
2024-04-24  5:28   ` Patrick Steinhardt
2024-04-24  3:58 ` [PATCH 2/2] advice: add "all" option to disable all hints James Liu
2024-04-24  5:29   ` Patrick Steinhardt
2024-04-24  6:28 ` [PATCH 0/2] " Junio C Hamano
2024-04-24  6:48   ` Patrick Steinhardt
2024-04-24 13:52     ` Phillip Wood
2024-04-24 14:07       ` Patrick Steinhardt
2024-04-24 14:59         ` Junio C Hamano
2024-04-25  6:46           ` Patrick Steinhardt
2024-04-25 16:18             ` Junio C Hamano
2024-04-24 16:14         ` Dragan Simic
2024-04-24 16:21           ` Dragan Simic
2024-04-24 14:31     ` Junio C Hamano
2024-04-25  6:42       ` Patrick Steinhardt
2024-04-24  7:37   ` Dragan Simic
2024-04-29  1:09 ` [PATCH v2 0/1] advice: add --no-advice global option James Liu
2024-04-29  1:09   ` [PATCH v2 1/1] " James Liu
2024-04-29  4:15     ` Dragan Simic
2024-04-29  5:01       ` James Liu
2024-04-29  5:36         ` Dragan Simic
2024-04-29  5:59           ` Dragan Simic
2024-04-29  6:04             ` Eric Sunshine
2024-04-29  6:12               ` Dragan Simic
2024-04-29  6:40         ` Jeff King
2024-04-29  6:55           ` Dragan Simic
2024-04-29 13:50           ` Junio C Hamano
2024-04-30  0:56           ` James Liu
2024-04-29 13:48       ` Junio C Hamano
2024-04-29 17:05     ` Rubén Justo
2024-04-29 17:54       ` Dragan Simic
2024-04-30  1:47   ` [PATCH v3 0/1] " James Liu
2024-04-30  1:47     ` [PATCH v3 1/1] " James Liu
2024-04-30  5:18       ` Patrick Steinhardt
2024-04-30  6:24         ` Dragan Simic
2024-04-30 16:29       ` Junio C Hamano
2024-05-03  7:17     ` [PATCH v4 0/3] advice: add "all" option to disable all hints James Liu
2024-05-03  7:17       ` [PATCH v4 1/3] doc: clean up usage documentation for --no-* opts James Liu
2024-05-03 17:30         ` Junio C Hamano
2024-05-06  1:39           ` James Liu
2024-05-03  7:17       ` [PATCH v4 2/3] doc: add spacing around paginate options James Liu
2024-05-03 14:32         ` Karthik Nayak
2024-05-03 17:36           ` Junio C Hamano
2024-05-03  7:17       ` [PATCH v4 3/3] advice: add --no-advice global option James Liu
2024-05-08  0:40         ` [PATCH v4 4/3] t0018: two small fixes Junio C Hamano
2024-05-03  7:31       ` [PATCH v4 0/3] advice: add "all" option to disable all hints Dragan Simic
2024-05-03 18:00         ` Re* " Junio C Hamano
2024-05-03 19:26           ` Eric Sunshine
2024-05-03 19:48             ` Junio C Hamano
2024-05-03 20:08               ` Junio C Hamano
2024-05-03 21:24                 ` Eric Sunshine
2024-05-05 11:03                   ` Johannes Schindelin
2024-05-06 16:40                     ` Junio C Hamano [this message]
2024-05-07  0:11                       ` Re* " Dragan Simic
2024-05-07  0:21                         ` Junio C Hamano
2024-05-07  4:45                           ` Dragan Simic
2024-05-07  0:01                   ` Dragan Simic
2024-05-03 14:35       ` Karthik Nayak
2024-05-05 23:17         ` James Liu
2024-05-03 17:25       ` Junio C Hamano
2024-05-05 23:20         ` James Liu
2024-05-06  1:10           ` James Liu
2024-05-06 16:47             ` Junio C Hamano
2024-05-06 23:08               ` James Liu
2024-05-07  6:23                 ` Karthik Nayak
2024-05-06 16:41           ` Junio C Hamano

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=xmqqbk5i3ncw.fsf_-_@gitster.g \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=dsimic@manjaro.org \
    --cc=git@vger.kernel.org \
    --cc=james@jamesliu.io \
    --cc=sunshine@sunshineco.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 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 NNTP newsgroup(s).