| Message ID | 20190129193818.8645-6-jeremyhu@apple.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Differences between git-2.20.1 and Apple Git-116 | expand |
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. 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. > --- > 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 Ciao, Johannes > ls-refs > fetch=shallow > server-option > -- > 2.20.0 (Apple Git-115) > >
> 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. > 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 > https://pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html Also done, thanks. > > Ciao, > Johannes > >> ls-refs >> fetch=shallow >> server-option >> -- >> 2.20.0 (Apple Git-115) >> >>
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") ls-refs fetch=shallow server-option
Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com> --- t/t5701-git-serve.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)