Re: [PATCH (Apple Git) 05/13] t5701: git --version can have SP in it
- Date: Wed, 30 Jan 2019 11:35:05 -0800
- From: Jeremy Huddleston Sequoia <jeremyhu@xxxxxxxxx>
- Subject: Re: [PATCH (Apple Git) 05/13] t5701: git --version can have SP in it
> On Jan 30, 2019, at 05:36, Johannes Schindelin <Johannes.Schindelin@xxxxxx> wrote:
> Hi Jeremy,
> On Tue, 29 Jan 2019, Jeremy Huddleston Sequoia wrote:
>> Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@xxxxxxxxx>
> 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.
> 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.
>> 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,
Good catch, thanks.
> and it would be much conciser to
> use `-e "y/ /./"`, which even BSD sed understands according to
Also done, thanks.
>> 2.20.0 (Apple Git-115)