From: Eric Sunshine <sunshine@sunshineco.com>
Date: Fri, 25 Mar 2016 00:05:55 GMT
Subject: Re: [PATCH v9 3/3] commit: add a commit.verbose config variable
Message-ID: <CAPig+cQ3TnKV1GcViq9Qwtq8bkp19jqvQRzBT_v5-MSfSfXgZA@mail.gmail.com>
In-Reply-To: <01020153a7ba4ec2-1fb668c3-9dc6-4137-8d92-c6a5825a156c-000000@eu-west-1.amazonses.com>

On Thu, Mar 24, 2016 at 4:25 AM, Pranit Bauva <pranit.bauva@gmail.com> wrote:
> Add commit.verbose configuration variable as a convenience for those
> who always prefer --verbose.

The implementation looks better in this version. A couple comments
below about the test script...

> Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
> ---
> diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
> @@ -9,6 +9,12 @@ test $(wc -l <out) = 1
>  EOF
>  chmod +x check-for-diff

Mentioned in patch 2/3 review, this patch (3/3) is where you should
update 'check-for-diff' to also check the line count, and the commit
message should explain the reason for doing so (and don't forget to
mention that it won't harm existing clients of 'check-for-diff').

> +write_script "check-for-double-diff" <<-\EOF &&
> +grep '# Changes not staged for commit' "$1" >out &&
> +test $(wc -l <out) = 2
> +EOF
> +chmod +x check-for-double-diff

Also mentioned in patch 2/3 review: drop 'chmod'; write_script() does
it for you.