{"thread":{"id":"41466","subject":"[PATCH] Update diff-highlight","startedAt":"2016-02-22T04:14:15Z","lastAt":"2016-02-26T09:41:41Z","messageCount":5,"participants":["Peter Dave Hello","Eric Sunshine","Junio C Hamano","Roberto Tyley"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"278829","messageId":"00000153072f5465-fd36f07c-e5e6-4a0f-8cf1-77be99424892-000000@eu-west-1.amazonses.com","threadId":"41466","inReplyTo":null,"subject":"[PATCH] Update diff-highlight","fromName":"Peter Dave Hello","fromEmail":"hsu@peterdavehello.org","sentAt":"2016-02-22T04:14:15Z","receivedAt":"2016-02-22T04:14:15Z","isPatch":true,"sender":{"key":"hsu@peterdavehello.org","avatar":"https://gravatar.com/avatar/f2a327ef8e09f04a35ebae7059f870b6281779441fad3d84b86ca9eef53e513b?d=mp&s=160"},"body":"From: Peter Dave Hello \n\nUse `#!/usr/bin/env perl` instead of `#!/usr/bin/perl`\n\nSo that it can works on FreeBSD.\n---\n contrib/diff-highlight/diff-highlight | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/diff-highlight/diff-highlight b/contrib/diff-highlight/diff-highlight\nindex ffefc31..b57b0fd 100755\n--- a/contrib/diff-highlight/diff-highlight\n+++ b/contrib/diff-highlight/diff-highlight\n@@ -1,4 +1,4 @@\n-#!/usr/bin/perl\n+#!/usr/bin/env perl\n \n use 5.008;\n use warnings FATAL => 'all';\n\n--\nhttps://github.com/git/git/pull/200\n"},{"id":"278830","messageId":"CAPig+cRRX4k8jR02YOGTGHUnbOmhHF0oMc-6aCOWuMOpi+BG0A@mail.gmail.com","threadId":"41466","inReplyTo":"00000153072f5465-fd36f07c-e5e6-4a0f-8cf1-77be99424892-000000@eu-west-1.amazonses.com","subject":"Re: [PATCH] Update diff-highlight","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-02-22T04:49:03Z","receivedAt":"2016-02-22T04:49:03Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Feb 21, 2016 at 11:14 PM, Peter Dave Hello\n wrote:\n> From: Peter Dave Hello \n\nThis \"From:\" line looks suspiciously incorrect. If anything, you'd\nprobably want to drop the line altogether or use:\n\n From: Peter Dave Hello \n\n> Update diff-highlight\n\nPatches do indeed \"update\" the project, but this summary line isn't\ntelling us much about intention of this patch. Perhaps rephrase it as:\n\n contrib/diff-highlight: stop hard-coding perl location\n\n> Use `#!/usr/bin/env perl` instead of `#!/usr/bin/perl`\n>\n> So that it can works on FreeBSD.\n\ns/works/work/\n\nAlso, you probably want to combine those two lines into one proper\nsentence rather than having one sentence plus a sentence fragment.\n\nYour Signed-off-by: is missing.\n\nThanks.\n\n> ---\n> contrib/diff-highlight/diff-highlight | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/contrib/diff-highlight/diff-highlight b/contrib/diff-highlight/diff-highlight\n> index ffefc31..b57b0fd 100755\n> --- a/contrib/diff-highlight/diff-highlight\n> +++ b/contrib/diff-highlight/diff-highlight\n> @@ -1,4 +1,4 @@\n> -#!/usr/bin/perl\n> +#!/usr/bin/env perl\n>\n> use 5.008;\n> use warnings FATAL => 'all';\n>\n> --\n> https://github.com/git/git/pull/200\n"},{"id":"278835","messageId":"CA+boQ6Y4223NY_mXHPNR7hpxiepgoSmbM8c6bJxirxTMDVQ8Bg@mail.gmail.com","threadId":"41466","inReplyTo":"CAPig+cRRX4k8jR02YOGTGHUnbOmhHF0oMc-6aCOWuMOpi+BG0A@mail.gmail.com","subject":"Re: [PATCH] Update diff-highlight","fromName":"Peter Dave Hello","fromEmail":"hsu@peterdavehello.org","sentAt":"2016-02-22T05:59:40Z","receivedAt":"2016-02-22T05:59:40Z","isPatch":true,"sender":{"key":"hsu@peterdavehello.org","avatar":"https://gravatar.com/avatar/f2a327ef8e09f04a35ebae7059f870b6281779441fad3d84b86ca9eef53e513b?d=mp&s=160"},"body":"Hello Eric,\n\nThanks for your review and prompt reply, this is my first PR to git,\nI'll try to update it to follow the conventions.\n\nBest,\nPeter\n\n--\n\nNow you can follow me on twitter or GitHub :D\n\n\n2016-02-22 12:49 GMT+08:00 Eric Sunshine :\n> On Sun, Feb 21, 2016 at 11:14 PM, Peter Dave Hello\n> wrote:\n>> From: Peter Dave Hello \n>\n> This \"From:\" line looks suspiciously incorrect. If anything, you'd\n> probably want to drop the line altogether or use:\n>\n> From: Peter Dave Hello \n>\n>> Update diff-highlight\n>\n> Patches do indeed \"update\" the project, but this summary line isn't\n> telling us much about intention of this patch. Perhaps rephrase it as:\n>\n> contrib/diff-highlight: stop hard-coding perl location\n>\n>> Use `#!/usr/bin/env perl` instead of `#!/usr/bin/perl`\n>>\n>> So that it can works on FreeBSD.\n>\n> s/works/work/\n>\n> Also, you probably want to combine those two lines into one proper\n> sentence rather than having one sentence plus a sentence fragment.\n>\n> Your Signed-off-by: is missing.\n>\n> Thanks.\n>\n>> ---\n>> contrib/diff-highlight/diff-highlight | 2 +-\n>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/contrib/diff-highlight/diff-highlight b/contrib/diff-highlight/diff-highlight\n>> index ffefc31..b57b0fd 100755\n>> --- a/contrib/diff-highlight/diff-highlight\n>> +++ b/contrib/diff-highlight/diff-highlight\n>> @@ -1,4 +1,4 @@\n>> -#!/usr/bin/perl\n>> +#!/usr/bin/env perl\n>>\n>> use 5.008;\n>> use warnings FATAL => 'all';\n>>\n>> --\n>> https://github.com/git/git/pull/200\n"},{"id":"278842","messageId":"xmqqh9h189ph.fsf@gitster.mtv.corp.google.com","threadId":"41466","inReplyTo":"00000153072f5465-fd36f07c-e5e6-4a0f-8cf1-77be99424892-000000@eu-west-1.amazonses.com","subject":"Re: [PATCH] Update diff-highlight","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-22T07:50:50Z","receivedAt":"2016-02-22T07:50:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Peter Dave Hello writes:\n\n> From: Peter Dave Hello \n>\n> Use `#!/usr/bin/env perl` instead of `#!/usr/bin/perl`\n\nEven though there are existing examples in contrib/ parts that use\nthis pattern, we try to avoid use of #!/usr/bin/env in more serious\nparts of our system. This is to control the exact interpreter used\nto run our scripts at the build time, and to avoid interference by\nend users' $PATH environment. So adding more use of #!/usr/bin/env\nis not quite a welcome move, even if it is to contrib/ part.\n\nPerhaps you can instead mimick the way how contrib/subtree uses the\nsame SHELL_PATH used in the primary Makefile (and config.mak) to\nturn git-subtree.sh into git-subtree command. Rename the source to\ndiff-highlight.perl, and use PERL_PATH when build procedure turns it\ninto diff-highlight?\n\nI think that is more in line with the rest of the system.\n"},{"id":"279508","messageId":"CAFY1edZ=zs-MyVFmejy32DUyvCjPeb9Kt9J0bFEqVOQMQgW_Og@mail.gmail.com","threadId":"41466","inReplyTo":"CAPig+cRRX4k8jR02YOGTGHUnbOmhHF0oMc-6aCOWuMOpi+BG0A@mail.gmail.com","subject":"Re: [PATCH] Update diff-highlight","fromName":"Roberto Tyley","fromEmail":"roberto.tyley@gmail.com","sentAt":"2016-02-26T09:41:41Z","receivedAt":"2016-02-26T09:41:41Z","isPatch":true,"sender":{"key":"roberto.tyley@gmail.com","avatar":"https://avatars.githubusercontent.com/u/52038?v=4"},"body":"On 22 February 2016 at 04:49, Eric Sunshine wrote:\n> On Sun, Feb 21, 2016 at 11:14 PM, Peter Dave Hello\n> wrote:\n>> From: Peter Dave Hello \n>\n> This \"From:\" line looks suspiciously incorrect. If anything, you'd\n> probably want to drop the line altogether or use:\n>\n> From: Peter Dave Hello \n\nPeter's commit (https://github.com/git/git/commit/15415c6e) had an author of\n'peterdavehello@users.noreply.github.com' (perhaps because the commit was\ngenerated through GitHub's interface?), and submitGit added it as an in-body\n'From: ' line because it differed from the address used to send the email\n(hsu@peterdavehello.org - submitGit always uses the user's\nprimary-email-address-in-GitHub to send the email).\n\nA 'noreply' address is obviously not wanted in this context though, so\nI've updated\nsubmitGit to disregard them when deciding whether or not to generate an in-body\n'From: ' header: https://github.com/rtyley/submitgit/pull/29\n\nRoberto\n"}]}