Re: [PATCH v2 1/3] [Outreachy] t3903-stash: test without configured user name

From: Junio C Hamano <gitster@pobox.com>
To: Eric Sunshine <sunshine@sunshineco.com>
Cc: slavicadj.ip2018@gmail.com,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Git List <git@vger.kernel.org>, Slavica <slawica92@hotmail.com>
Subject: Re: [PATCH v2 1/3] [Outreachy] t3903-stash: test without configured user name
Date: Thu, 25 Oct 2018 13:48:33 +0900	[thread overview]
Message-ID: <xmqqtvlak4wu.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <CAPig+cRGn0Z7F7TpSwF=8cQJpN1LJkQb2VxHMDi6j6wsaqkORg@mail.gmail.com> (Eric Sunshine's message of "Wed, 24 Oct 2018 16:25:55 -0400")

Eric Sunshine <sunshine@sunshineco.com> writes:

>> +    test_commit 1 &&
>> +    test_config user.useconfigonly true &&
>> +    test_config stash.usebuiltin true &&
>> +    sane_unset GIT_AUTHOR_NAME &&
>> +    sane_unset GIT_AUTHOR_EMAIL &&
>> +    sane_unset GIT_COMMITTER_NAME &&
>> +    sane_unset GIT_COMMITTER_EMAIL &&
>> +    test_must_fail git config user.email &&
>
> Instead of simply asserting that 'user.email' is not set here, you
> could instead proactively ensure that it is not set. That is, instead
> of the test_must_fail(), do this:
>
>     test_unconfig user.email &&
>     test_unconfig user.name &&

Yes, it would be more in line with what is done to the environment
variables and to other configuration variables in the same block.

Not that I think that this inconsistency is end of the world ;-)

Thanks.

>> +    echo changed >1.t &&
>> +    git stash
>> +'
>> +
>>  test_done
>> --
>> 2.19.1.windows.1
>>