Message ID | 20220728174953.66964-1-sunshine@sunshineco.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | format-patch: clarify --creation-factor=<factor> | expand |
On Thu, Jul 28 2022, Eric Sunshine wrote: > The value is not a percentage that ranges from 0 to 100, so stop > referring to it as `percent`; instead follow the lead of the `git > range-diff` documentation and call it `factor`. > > Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> > --- > > This is a sibling to Junio's patch[1]. > > [1]: https://lore.kernel.org/git/xmqqo7x9ch7n.fsf_-_@gitster.g/ > > Documentation/git-format-patch.txt | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt > index be797d7a28..e06475abcd 100644 > --- a/Documentation/git-format-patch.txt > +++ b/Documentation/git-format-patch.txt > @@ -27,7 +27,7 @@ SYNOPSIS > [--[no-]encode-email-headers] > [--no-notes | --notes[=<ref>]] > [--interdiff=<previous>] > - [--range-diff=<previous> [--creation-factor=<percent>]] > + [--range-diff=<previous> [--creation-factor=<factor>]] > [--filename-max-length=<n>] > [--progress] > [<common diff options>] > @@ -321,7 +321,7 @@ product of `format-patch` is generated, and they are not passed to > the underlying `range-diff` machinery used to generate the cover-letter > material (this may change in the future). > > ---creation-factor=<percent>:: > +--creation-factor=<factor>:: > Used with `--range-diff`, tweak the heuristic which matches up commits > between the previous and current series of patches by adjusting the > creation/deletion cost fudge factor. See linkgit:git-range-diff[1]) Looks good as far as it goes, looks like both of your patches need to also tweak this bit though: $ git -P grep 'percentage.*creation' -- '*.c' builtin/log.c: N_("percentage by which creation is weighted")), builtin/range-diff.c: N_("percentage by which creation is weighted")), Probably just s/percentage/factor/ in for those -h strings?
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: >> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt >> index be797d7a28..e06475abcd 100644 >> --- a/Documentation/git-format-patch.txt >> +++ b/Documentation/git-format-patch.txt >> @@ -27,7 +27,7 @@ SYNOPSIS >> [--[no-]encode-email-headers] >> [--no-notes | --notes[=<ref>]] >> [--interdiff=<previous>] >> - [--range-diff=<previous> [--creation-factor=<percent>]] >> + [--range-diff=<previous> [--creation-factor=<factor>]] >> [--filename-max-length=<n>] >> [--progress] >> [<common diff options>] >> @@ -321,7 +321,7 @@ product of `format-patch` is generated, and they are not passed to >> the underlying `range-diff` machinery used to generate the cover-letter >> material (this may change in the future). >> >> ---creation-factor=<percent>:: >> +--creation-factor=<factor>:: >> Used with `--range-diff`, tweak the heuristic which matches up commits >> between the previous and current series of patches by adjusting the >> creation/deletion cost fudge factor. See linkgit:git-range-diff[1]) > > Looks good as far as it goes, looks like both of your patches need to > also tweak this bit though: > > $ git -P grep 'percentage.*creation' -- '*.c' > builtin/log.c: N_("percentage by which creation is weighted")), > builtin/range-diff.c: N_("percentage by which creation is weighted")), > > Probably just s/percentage/factor/ in for those -h strings? Thanks for being extra careful. Eric, I am not sure if the other patch for the range-diff command as a standalone patch is worth it. Perhaps you can help me by submitting a single combined patch to cover the above as well as what we wrote in the two patches, with you marked as the primary author and with Helped-by: that credits Ævar? Thanks.
On Thu, Jul 28, 2022 at 4:59 PM Junio C Hamano <gitster@pobox.com> wrote: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > >> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt > >> @@ -27,7 +27,7 @@ SYNOPSIS > >> - [--range-diff=<previous> [--creation-factor=<percent>]] > >> + [--range-diff=<previous> [--creation-factor=<factor>]] > >> @@ -321,7 +321,7 @@ product of `format-patch` is generated, and they are not passed to > >> ---creation-factor=<percent>:: > >> +--creation-factor=<factor>:: > > > > Looks good as far as it goes, looks like both of your patches need to > > also tweak this bit though: > > > > $ git -P grep 'percentage.*creation' -- '*.c' > > builtin/log.c: N_("percentage by which creation is weighted")), > > builtin/range-diff.c: N_("percentage by which creation is weighted")), > > > > Probably just s/percentage/factor/ in for those -h strings? > > Thanks for being extra careful. > > Eric, I am not sure if the other patch for the range-diff command as > a standalone patch is worth it. Perhaps you can help me by > submitting a single combined patch to cover the above as well as > what we wrote in the two patches, with you marked as the primary > author and with Helped-by: that credits Ævar? Will do. (Slightly chagrined that I forgot to check the usage-string in the C files. Thanks, Ævar.)
On Thu, Jul 28, 2022 at 5:09 PM Eric Sunshine <sunshine@sunshineco.com> wrote: > On Thu, Jul 28, 2022 at 4:59 PM Junio C Hamano <gitster@pobox.com> wrote: > > >> - [--range-diff=<previous> [--creation-factor=<percent>]] > > >> + [--range-diff=<previous> [--creation-factor=<factor>]] > > > > Eric, I am not sure if the other patch for the range-diff command as > > a standalone patch is worth it. Perhaps you can help me by > > submitting a single combined patch to cover the above as well as > > what we wrote in the two patches, with you marked as the primary > > author and with Helped-by: that credits Ævar? > > Will do. Given Dscho's response in [1] which states that "percent" is more accurate than "factor", perhaps these two patches are unwanted after all, and instead the documentation of "creation factor" in Documentation/git-range-diff.txt ought to be expanded with a better explanation, as hinted in [2]. [1]: https://lore.kernel.org/git/85snn12q-po05-osqs-n1o0-n6040392q01q@tzk.qr/ [2]: https://lore.kernel.org/git/xmqq5yjf4l60.fsf@gitster.g/
Eric Sunshine <sunshine@sunshineco.com> writes: > On Thu, Jul 28, 2022 at 5:09 PM Eric Sunshine <sunshine@sunshineco.com> wrote: >> On Thu, Jul 28, 2022 at 4:59 PM Junio C Hamano <gitster@pobox.com> wrote: >> > >> - [--range-diff=<previous> [--creation-factor=<percent>]] >> > >> + [--range-diff=<previous> [--creation-factor=<factor>]] >> > >> > Eric, I am not sure if the other patch for the range-diff command as >> > a standalone patch is worth it. Perhaps you can help me by >> > submitting a single combined patch to cover the above as well as >> > what we wrote in the two patches, with you marked as the primary >> > author and with Helped-by: that credits Ævar? >> >> Will do. > > Given Dscho's response in [1] which states that "percent" is more > accurate than "factor", perhaps these two patches are unwanted after > all, and instead the documentation of "creation factor" in > Documentation/git-range-diff.txt ought to be expanded with a better > explanation, as hinted in [2]. Yup, I do not mind "percent" at all. As long as it is clear that we use it to avoid fractions by multiplying by 100. Something along the lines of "This number is expressed by 'percent', but its range is not constrained to 0-100%. It is a weight given to X relative to Y, and default is 60% (i.e. 0.6). You give it larger value when you want to do Z and smaller value when you want to do W" would be what our readers need to see, I would think. Thanks.
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt index be797d7a28..e06475abcd 100644 --- a/Documentation/git-format-patch.txt +++ b/Documentation/git-format-patch.txt @@ -27,7 +27,7 @@ SYNOPSIS [--[no-]encode-email-headers] [--no-notes | --notes[=<ref>]] [--interdiff=<previous>] - [--range-diff=<previous> [--creation-factor=<percent>]] + [--range-diff=<previous> [--creation-factor=<factor>]] [--filename-max-length=<n>] [--progress] [<common diff options>] @@ -321,7 +321,7 @@ product of `format-patch` is generated, and they are not passed to the underlying `range-diff` machinery used to generate the cover-letter material (this may change in the future). ---creation-factor=<percent>:: +--creation-factor=<factor>:: Used with `--range-diff`, tweak the heuristic which matches up commits between the previous and current series of patches by adjusting the creation/deletion cost fudge factor. See linkgit:git-range-diff[1])
The value is not a percentage that ranges from 0 to 100, so stop referring to it as `percent`; instead follow the lead of the `git range-diff` documentation and call it `factor`. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> --- This is a sibling to Junio's patch[1]. [1]: https://lore.kernel.org/git/xmqqo7x9ch7n.fsf_-_@gitster.g/ Documentation/git-format-patch.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)