Re: [WIP PATCH/RFC] Use a higher range-diff --creation-factor for format-patch

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mar 21, 2019, 11:22 UTC
Message-ID: <nycvar.QRO.7.76.6.1903211209280.41@tvgsbejvaqbjf.bet>
In-Reply-To: <87tvg49c5u.fsf@evledraar.gmail.com>

Hi Ævar and Eric,

On Fri, 15 Mar 2019, Ævar Arnfjörð Bjarmason wrote:

Show 36 quoted lines

>
> On Fri, Mar 15 2019, Eric Sunshine wrote:
>
> > On Fri, Mar 15, 2019 at 12:09 PM Ævar Arnfjörð Bjarmason
> > <avarab@gmail.com> wrote:
> >> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
> >> @@ -261,6 +261,10 @@ material (this may change in the future).
> >> +Defaults to 90, whereas the linkgit:git-range-diff[1] default is
> >> +60. It's assumed that you're submitting a new patch series & that we
> >> +should try harder than normal to find similarities.
> >
> > My understanding was that the primary use-case of git-range-diff
> > itself (which existed before the --range-diff option was added to
> > git-format-patch) was to generate a "range diff" for a cover letter of
> > a re-rolled series. So, I'm confused about why this tweaks the default
> > value of one command but not the other.
> >
> >> diff --git a/range-diff.h b/range-diff.h
> >> @@ -4,6 +4,7 @@
> >>  #define RANGE_DIFF_CREATION_FACTOR_DEFAULT 60
> >> +#define RANGE_DIFF_CREATION_FACTOR_FORMAT_PATCH_DEFAULT 90
> >
> > The point of introducing RANGE_DIFF_CREATION_FACTOR_DEFAULT in the
> > first place was to ensure that the default creation-factor didn't get
> > out-of-sync between git-range-diff and git-format-patch., Thus,
> > introducing this sort of inconsistency between the two would likely
> > lead to confusion on the part of users. After all, --range-diff was
> > added to git-format-patch merely as a convenience over having to run
> > git-range-diff separately and copy/pasting its output into a cover
> > letter generated by git-format-patch. If the two programs default to
> > different values, then that "convenience equality" breaks down.
> >
> > So, I'm fairly negative on this change. However, that doesn't mean I
> > would oppose tweaking the value shared between the two programs (and
> > also the default used by GitGitGadget, if it specifies it manually),
> > though I defer to Dscho in that regard.

GitGitGadget does not specify the creation factor manually.

> I think that was the intention initially, but at least I'm now using
> range-diff as a general comparison tool of different non-ff-branches,
> e.g. the force-pushes to "pu".

Interesting ;-)

Show 6 quoted lines

> I'd expect the tool in general to be used like that, whereas with
> format-patch it's safe to say we're dealing with a re-roll of the same
> thing.
>
> Hence the hypothesis that for format-patch we can be more aggressive
> about finding similarities.

I do agree that `format-patch`'s `--range-diff` is certainly exclusively used for comparing different iterations of the same patch series. As such, I do agree with Ævar that it makes sense to have a *different* default for the creation factor.

Having said that, I did notice while porting `tbdiff` to C that it would be a neat idea to put more weight behind the differences of the commit message, and maybe even use a *different* measure on the commit message, too. Personally, I would try to use a variation of the word diff (maybe something that reflects my experience that it is common to change the case in the oneline, to add a sentence here and there, or to delete a sentence, but not so much to replace entire sentences), to account for the fact that the commit message rarely changes substantially between iterations.

So I guess there is a lot of room for improvement here: code (read: diff) changes simply are not the same as commit message changes.

Ciao, Dscho