From: Eric Sunshine <sunshine@sunshineco.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: 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: Fri, 3 May 2024 15:26:48 -0400 [thread overview]
Message-ID: <CAPig+cRZEQjwhnMYvQ_0BjvXaR=3x=LQeJHzFOxC+ZPyOy9yJQ@mail.gmail.com> (raw)
In-Reply-To: <xmqqjzka7p2t.fsf_-_@gitster.g>
On Fri, May 3, 2024 at 2:00 PM Junio C Hamano <gitster@pobox.com> wrote:
> Dragan Simic <dsimic@manjaro.org> writes:
> > Just a small suggestion... Perhaps the creation factor needs adjusting
> > for the range diff to actually be produced. [...]
>
> I see this happen to too many series I see on the list. There are
> cases when the user knows that they are comparing an old and a new
> iterations of the same series, e.g. running it from format-patch.
> We probably should use a much higher creation factor than default to
> run range-diff in such a context.
>
> IOW, this shouldn't have to be done by individual users, but by the
> tool.
>
> Perhaps something along this line may not be a bad idea.
>
> ----- >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>
Ævar had posted[1] pretty much the exact same patch a few years ago.
At the time, I had trouble understanding why `git range-diff` and `git
format-patch --range-dif` would need separate default creation
factors[2], and I still have trouble understanding why they should be
different. Aren't both commands addressing the same use-case of
comparing one version of a series against a subsequent version? In
your response[3], you seemed to agree with that observation and
suggested instead simply increasing the default creation factor for
both commands (which sounds reasonable to me).
[1]: https://lore.kernel.org/git/87y35g9l18.fsf@evledraar.gmail.com/
[2]: https://lore.kernel.org/git/CAPig+cRMiEcXVRYrgp+B3tcDreh41-a5_k0zABe+HNce0G=Cyw@mail.gmail.com/
[3]: https://lore.kernel.org/git/xmqqzhps4uyq.fsf@gitster-ct.c.googlers.com/
> ---
> diff --git c/builtin/log.c w/builtin/log.c
> index 4da7399905..7a019476c3 100644
> --- c/builtin/log.c
> +++ w/builtin/log.c
> @@ -2294,7 +2294,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 c/range-diff.h w/range-diff.h
> index 04ffe217be..2f69f6a434 100644
> --- c/range-diff.h
> +++ w/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
next prev parent reply other threads:[~2024-05-03 19:27 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 [this message]
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 ` Re* " Junio C Hamano
2024-05-07 0:11 ` 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='CAPig+cRZEQjwhnMYvQ_0BjvXaR=3x=LQeJHzFOxC+ZPyOy9yJQ@mail.gmail.com' \
--to=sunshine@sunshineco.com \
--cc=dsimic@manjaro.org \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=james@jamesliu.io \
/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 read-only IMAP folder(s) and NNTP newsgroup(s).