Re: [PATCH v7 2/2] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed

[Eric Sunshine <sunshine@sunshineco.com>]

On Fri, Feb 05, 2016 at 11:29:06PM +0200, Dan Aloni wrote:
> It used to be that:
> 
>    git config --global user.email "(none)"
> 
> was a viable way for people to force themselves to set user.email in
> each repository.  This was helpful for people with more than one
> email address, targeting different email addresses for different
> clones, as it barred git from creating commit unless the user.email

Either: s/commit/a commit/ or s/commit/commits/

> config was set in the per-repo config to the correct email address.
> 
> A recent change, 19ce497c (ident: keep a flag for bogus
> default_email, 2015-12-10), however declared that an explicitly

s/however/&,/

> configured user.email is not bogus, no matter what its value is, so
> this hack no longer works.
> 
> Provide the same functionality by adding a new configuration
> variable user.useConfigOnly; when this variable is set, the
> user must explicitly set user.email configuration.
> 
> Signed-off-by: Dan Aloni <alonid@gmail.com>
> Helped-by: Jeff King <peff@peff.net>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> Cc: Eric Sunshine <sunshine@sunshineco.com>

You'd generally place your sign-off last.

> ---
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> @@ -2821,6 +2821,16 @@ user.name::
> +user.useConfigOnly::
> +	This instructs Git to avoid trying to guess defaults for 'user.email'

Perhaps: s/This instructs/Instruct/

> +	and 'user.name' other than strictly from config. For example, if

The way this is phrased, it sounds almost as if Git also "guesses"
the value from config. Perhaps rephrase like this:

    Instruct Git to avoid trying to guess defaults for 'user.email'
    and 'user.name', and instead retrieve the values only from
    configuration.

> +	you have multiple email addresses and would like to use a different
> +	one for each repository, then with this configuration option set
> +	to `true` in the global config along with a name, Git would prompt

s/would/will/

> +	for you for setting up an email upon making new commits in a newly

s/for you for setting/you to set/

More below...

> +	cloned repository.
> +	Defaults to `false`.
> diff --git a/t/t9904-per-repo-email.sh b/t/t9904-per-repo-email.sh
> @@ -0,0 +1,55 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2016 Dan Aloni
> +#
> +
> +test_description='per-repo forced setting of email address'
> +
> +. ./test-lib.sh
> +
> +reprepare () {
> +	git reset --hard initial
> +}
> +
> +test_expect_success setup '
> +	# Initial repo state
> +	echo "Initial" >foo &&
> +	git add foo &&
> +	git commit -m foo &&
> +	git tag initial &&
> +
> +	# Setup a likely user.useConfigOnly use case
> +	sane_unset GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL &&
> +	sane_unset GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL &&
> +	git config user.name "test" &&
> +	git config --global user.useConfigOnly true &&
> +
> +	reprepare
> +'
> +
> +test_expect_success 'fails committing if clone email is not set' '
> +	test_when_finished reprepare &&
> +
> +	test_must_fail git commit --allow-empty -m msg
> +'
> +
> +test_expect_success 'fails committing if clone email is not set, but EMAIL set' '
> +	test_when_finished reprepare &&
> +
> +	test_must_fail env EMAIL=test@fail.com git commit --allow-empty -m msg
> +'
> +
> +test_expect_success 'succeeds committing if clone email is set' '
> +	test_when_finished reprepare &&
> +
> +	test_config user.email "test@ok.com" &&
> +	git commit --allow-empty -m msg
> +'
> +
> +test_expect_success 'succeeds cloning if global email is not set' '
> +	test_when_finished reprepare &&
> +
> +	git clone . clone
> +'
> +
> +test_done

This test script still has a fair amount of unnecessary cruft in it
which obscures the important bits showing what you are really
testing. Below is a more concise version with the unnecessary stuff
removed:

--- 8< ---
#!/bin/sh
#
# Copyright (c) 2016 Dan Aloni
#

test_description='per-repo forced setting of email address'

. ./test-lib.sh

test_expect_success setup '
	sane_unset GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL &&
	sane_unset GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL &&
	git config user.name "test" &&
	git config --global user.useConfigOnly true
'

test_expect_success 'fails committing if clone email is not set' '
	test_must_fail git commit --allow-empty -m msg
'

test_expect_success 'fails committing if clone email is not set, but EMAIL set' '
	test_must_fail env EMAIL=test@fail.com git commit --allow-empty -m msg
'

test_expect_success 'succeeds committing if clone email is set' '
	git -c user.email=test@ok.com commit --allow-empty -m msg
'

test_expect_success 'succeeds cloning if global email is not set' '
	git clone . clone
'

test_done
--- 8< ---