Re: [PATCH (Apple Git) 05/13] t5701: git --version can have SP in it
Show 11 quoted lines
> On Jan 30, 2019, at 05:36, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>
> Hi Jeremy,
>
> On Tue, 29 Jan 2019, Jeremy Huddleston Sequoia wrote:
>
>> Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
>
>
> That commit message is again very short. Only because I remember the
> previous patch's commit message do I have a clue what this is about.
Yeah, I became lax with commit messages when I was under the impression that the git community was not interested engaging with Apple. I should have taken a pass through these commits before sending, but I half expected these patches to be ignored. I am delighted to see that I was wrong and will go back and provide more details in the ones that I feel are upstreamable like this one.
Show 8 quoted lines
> You definitely need to write something here about customized forks of Git
> adding suffixes including spaces to the Git version.
>
> And you will need to state where those spaces are converted to dots in
> Git's capability advertisement. The reason for this requirement: should
> that logic change at any stage in the future, your patch will fail,
> somebody will investigate and find this commit and *needs* a helpful
> commit message.
Thanks, I'll add you to the CC of this patch, so you can critique my wording in the updated message.
Show 17 quoted lines
>
>> ---
>> t/t5701-git-serve.sh | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/t/t5701-git-serve.sh b/t/t5701-git-serve.sh
>> index ae79c6bbc0..7bc25700fa 100755
>> --- a/t/t5701-git-serve.sh
>> +++ b/t/t5701-git-serve.sh
>> @@ -7,7 +7,7 @@ test_description='test git-serve and server commands'
>> test_expect_success 'test capability advertisement' '
>> cat >expect <<-EOF &&
>> version 2
>> - agent=git/$(git version | cut -d" " -f3)
>> + agent=git/$(git --version | sed -e "s/git version //" -e "s/ /\./g")
>
> This `git version` needs to be anchored,
> and it would be much conciser to
> use `-e "y/ /./"`, which even BSD sed understands according to
> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html
Show 11 quoted lines
>
> Ciao,
> Johannes
>
>> ls-refs
>> fetch=shallow
>> server-option
>> --
>> 2.20.0 (Apple Git-115)
>>
>>
Jeremy Huddleston Sequoia <jeremyhu@apple.com>