> On Oct 10, 2016, at 09:42, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Jeremy Huddleston Sequoia <jeremyhu@xxxxxxxxx> writes:
>
>> Regressed-in: 480871e09ed2e5275b4ba16b278681e5a8c122ae
>
> Please be considerate to future readers of "git log" to help them
> avoid mistakes earlier commits made that caused you troubles.
>
> This line by itself without any explanation of what was broken is
> quite useless as a commit message. What can the readers do? They'd
> go back and say "git show 480871e09" and then what? The test added
> or modified by the commit has been working quite well for others
> since it was made, so it is very likely the reader wouldn't be able
> to tell if anything is wrong in it.
>
> You would help if you said what is different in _your_ environment
> from others who have happily been running and passing this test for
> others to understand and learn from your fix. What is it?
>
> The "Adjust ... distro changes" in the title offers some hint but it
> could be more explicit. Please write something along this line
> instead.
>
> Subject: t4014: git --version can have SP in it
>
> 480871e09e ("format-patch: show base info before email
> signature", 2016-09-07) added a helper function to recreate the
> signature at the end of the e-mail, i.e. "-- " line followed by
> the version string of Git, using output from "git --version" and
> stripping everything before the last SP.
>
> Because the default Git version string looks like "git version
> 2.10.0-1-g480871e09e", this was mostly OK, but people can change
> this version string to arbitrary thing while compiling, which
> can break the assumption if they had SP in it. Notably, Apple
> ships modified Git with " (Apple Git-xx)" appended to its
> version number.
>
> Instead, come up with the version string by stripping the "git
> version " from the beginning.
Ok, I'll add that explanation to the commit message, thanks.
>
>> Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@xxxxxxxxx>
>
> Good.
>
>> CC: Josh Triplett <josh@xxxxxxxxxxxxxxxx>
>> CC: Junio C Hamano <gitster@xxxxxxxxx>
>
> Please don't do this in your log message. These belong to your
> e-mail headers, not here.
Sorry, we do that consistently on other projects during the review process, so git send-email picks them up and adds them automatically for each patch being sent out. It's quite useful, actually.
>
>> ---
>> t/t4014-format-patch.sh | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
>> index 8d90a6e..33f6940 100755
>> --- a/t/t4014-format-patch.sh
>> +++ b/t/t4014-format-patch.sh
>> @@ -754,7 +754,7 @@ test_expect_success 'format-patch --ignore-if-in-upstream HEAD' '
>> git format-patch --ignore-if-in-upstream HEAD
>> '
>>
>> -git_version="$(git --version | sed "s/.* //")"
>> +git_version="$(git --version | sed "s/git version //")"
>
> Anchor the fixed prefix to the beginning, so that we can protect
> ourselves from another distro that would add "git version" in the
> middle of their arbitrary versioning scheme ;-). I.e.
>
> sed "s/^git version //"
Ok.
>
>>
>> signature() {
>> printf "%s\n%s\n\n" "-- " "${1:-$git_version}"
<<attachment: smime.p7s>>