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

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Feb 5, 2016, 21:59 UTC
Message-ID: <20160205215952.GA4901@flurp.local>
In-Reply-To: <1454707746-18672-3-git-send-email-alonid@gmail.com>

On Fri, Feb 05, 2016 at 11:29:06PM +0200, Dan Aloni wrote:

Show 8 quoted lines

> 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/&,/

Show 11 quoted lines

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

Show 5 quoted lines

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

Show 59 quoted lines

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

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