From: Jeremiah Mahler <jmmahler@gmail.com>
Date: Mon, 26 May 2014 22:14:25 GMT
Subject: Re: [PATCH v3 5/5] commit: support commit.verbose and --no-verbose
Message-ID: <20140526221425.GA20637@hudson.localdomain>
In-Reply-To: <1401130586-93105-6-git-send-email-caleb@calebthompson.io>

Caleb,

On Mon, May 26, 2014 at 01:56:26PM -0500, Caleb Thompson wrote:
> Add a new configuration variable commit.verbose to implicitly pass
> `--verbose` to `git-commit`. Add `--no-verbose` to commit to negate that
> setting.
> 
> Signed-off-by: Caleb Thompson <caleb@calebthompson.io>
> ---
>  Documentation/config.txt               |  5 +++++
>  '
...
>  
> +test_expect_success 'commit shows verbose diff with set commit.verbose=true' '
> +	echo morecontent >>file &&
...
> +'
> +
> +test_expect_success 'commit --verbose overrides verbose=false' '
> +	echo evenmorecontent >>file &&
...
> +
> +test_expect_success 'commit does not show verbose diff with commit.verbose=false' '
> +	echo evenmorecontent >>file &&
...
> +'
> +
> +test_expect_success 'commit --no-verbose overrides commit.verbose=true' '
> +	echo evenmorecontent >>file &&
...
> +'
> +
>  

Some minor style nits...

Use a consistent naming convention for your tests.  verbose=false looks
different than commit.verbose=false at first glance.  Also, since
"commit.verbose=false" is an invalid syntax for a config option, I would
remove the '=' and just make it "commit.verbose false".

-- 
Jeremiah Mahler
jmmahler@gmail.com
http://github.com/jmahler