Hello Eric, Thanks for your review and prompt reply, this is my first PR to git, I'll try to update it to follow the conventions. Best, Peter -- Now you can follow me on twitter or GitHub :D 2016-02-22 12:49 GMT+08:00 Eric Sunshine <sunshine@xxxxxxxxxxxxxx>: > On Sun, Feb 21, 2016 at 11:14 PM, Peter Dave Hello > <hsu@xxxxxxxxxxxxxxxxxx> wrote: >> From: Peter Dave Hello <peterdavehello@xxxxxxxxxxxxxxxxxxxxxxxx> > > This "From:" line looks suspiciously incorrect. If anything, you'd > probably want to drop the line altogether or use: > > From: Peter Dave Hello <hsu@xxxxxxxxxxxxxxxxxx> > >> Update diff-highlight > > Patches do indeed "update" the project, but this summary line isn't > telling us much about intention of this patch. Perhaps rephrase it as: > > contrib/diff-highlight: stop hard-coding perl location > >> Use `#!/usr/bin/env perl` instead of `#!/usr/bin/perl` >> >> So that it can works on FreeBSD. > > s/works/work/ > > Also, you probably want to combine those two lines into one proper > sentence rather than having one sentence plus a sentence fragment. > > Your Signed-off-by: is missing. > > Thanks. > >> --- >> contrib/diff-highlight/diff-highlight | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/contrib/diff-highlight/diff-highlight b/contrib/diff-highlight/diff-highlight >> index ffefc31..b57b0fd 100755 >> --- a/contrib/diff-highlight/diff-highlight >> +++ b/contrib/diff-highlight/diff-highlight >> @@ -1,4 +1,4 @@ >> -#!/usr/bin/perl >> +#!/usr/bin/env perl >> >> use 5.008; >> use warnings FATAL => 'all'; >> >> -- >> https://github.com/git/git/pull/200 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html