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

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Oct 23, 2018, 19:19 UTC
Message-ID: <CAPig+cSeWhdKXBdwm6C5XTY0wjGsxBX9GUvPv5nWNZEQRirDUA@mail.gmail.com>
In-Reply-To: <20181023162941.3840-1-slawica92@hotmail.com>

On Tue, Oct 23, 2018 at 12:31 PM Slavica <slavicadj.ip2018@gmail.com> wrote:

> This is part of enhancement request that ask for `git stash` to work even if `user.name` is not configured.
> The issue is discussed here: https://public-inbox.org/git/87o9debty4.fsf@evledraar.gmail.com/T/#u.

As Christian mentioned already, it's best to try to describe the issue succinctly in the commit message so readers can understand it without chasing a link. For this simple case, it should be sufficient to explain that, due to an implementation detail, git-stash undesirably requires 'user.name' and 'user.email' to be set, but shouldn't.

Show 5 quoted lines

> Signed-off-by: Slavica <slawica92@hotmail.com>
> ---
> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> @@ -1156,4 +1156,21 @@ test_expect_success 'stash -- <subdir> works with binary files' '
> +test_expect_failure 'stash with HOME as non-existing directory' '

The purpose of this test is to demonstrate that git-stash has an undesirable requirement that 'user.name' and 'user.email' be set. The test title should reflect that. So, instead of talking about non-existent HOME (which is just an implementation detail of the test), a better test title would be something like "stash works when user.name and user.email are not set".

Show 7 quoted lines

> +    test_commit 1 &&
> +    test_config user.useconfigonly true &&
> +    test_config stash.usebuiltin true &&
> +    (
> +        HOME=$(pwd)/none &&
> +        export HOME &&
> +        unset GIT_AUTHOR_NAME &&

Use sane_unset() for all of these rather than bare 'unset'.

Show 6 quoted lines

> +        unset GIT_AUTHOR_EMAIL &&
> +        unset GIT_COMMITTER_NAME &&
> +        unset GIT_COMMITTER_EMAIL &&
> +        test_must_fail git config user.email &&
> +        echo changed >1.t &&
> +               git stash

Christian already mentioned the odd indentation.

> +    )
> +'