Re: [PATCH] Update diff-highlight

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Feb 22, 2016, 04:49 UTC
Message-ID: <CAPig+cRRX4k8jR02YOGTGHUnbOmhHF0oMc-6aCOWuMOpi+BG0A@mail.gmail.com>
In-Reply-To: <00000153072f5465-fd36f07c-e5e6-4a0f-8cf1-77be99424892-000000@eu-west-1.amazonses.com>

On Sun, Feb 21, 2016 at 11:14 PM, Peter Dave Hello <hsu@peterdavehello.org> wrote:

> From: Peter Dave Hello <peterdavehello@users.noreply.github.com>

This "From:" line looks suspiciously incorrect. If anything, you'd probably want to drop the line altogether or use:

    From: Peter Dave Hello <hsu@peterdavehello.org>

> 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.

Show 17 quoted lines

> ---
>  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