> Subject: Re: [GSoC][PATCH v7 04/26] stash: renamed test cases to be more
> descriptive
Please use the imperative mood in the title and the commit messages
themselves. From Documentation/SubmittingPatches:
Describe your changes in imperative mood, e.g. "make xyzzy do frotz"
instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy
to do frotz", as if you are giving orders to the codebase to change
its behavior.
>From a quick skim over the rest of the series, this also applies to
some of the subsequent patches in the series.
On 08/08, Paul-Sebastian Ungureanu wrote:
> Renamed some test cases' labels to be more descriptive and under 80
> characters per line.
>
> Signed-off-by: Paul-Sebastian Ungureanu <[email protected]>
> ---
> t/t3903-stash.sh | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> index de6cab1fe..8d002a7f2 100755
> --- a/t/t3903-stash.sh
> +++ b/t/t3903-stash.sh
> @@ -604,7 +604,7 @@ test_expect_success 'stash show -p - no stashes on stack,
> stash-like argument' '
> test_cmp expected actual
> '
>
> -test_expect_success 'stash drop - fail early if specified stash is not a
> stash reference' '
> +test_expect_success 'drop: fail early if specified stash is not a stash ref'
> '
> git stash clear &&
> test_when_finished "git reset --hard HEAD && git stash clear" &&
> git reset --hard &&
> @@ -618,7 +618,7 @@ test_expect_success 'stash drop - fail early if specified
> stash is not a stash r
> git reset --hard HEAD
> '
>
> -test_expect_success 'stash pop - fail early if specified stash is not a
> stash reference' '
> +test_expect_success 'pop: fail early if specified stash is not a stash ref' '
> git stash clear &&
> test_when_finished "git reset --hard HEAD && git stash clear" &&
> git reset --hard &&
> @@ -682,7 +682,7 @@ test_expect_success 'invalid ref of the form "n", n >= N'
> '
> git stash drop
> '
>
> -test_expect_success 'stash branch should not drop the stash if the branch
> exists' '
> +test_expect_success 'branch: should not drop the stash if the branch exists'
> '
Since we're adjusting the titles of the tests here I'll allow myself
to nitpick a little :)
Maybe "branch: do not drop the stash if the branch exists", which
sounds more like an assertion, as the "pop" and "drop" titles above.
> git stash clear &&
> echo foo >file &&
> git add file &&
> @@ -693,7 +693,7 @@ test_expect_success 'stash branch should not drop the
> stash if the branch exists
> git rev-parse stash@{0} --
> '
>
> -test_expect_success 'stash branch should not drop the stash if the apply
> fails' '
> +test_expect_success 'branch: should not drop the stash if the apply fails' '
> git stash clear &&
> git reset HEAD~1 --hard &&
> echo foo >file &&
> @@ -707,7 +707,7 @@ test_expect_success 'stash branch should not drop the
> stash if the apply fails'
> git rev-parse stash@{0} --
> '
>
> -test_expect_success 'stash apply shows status same as git status (relative
> to current directory)' '
> +test_expect_success 'apply: shows same status as git status (relative to
> ./)' '
s/shows/show/ above maybe? This used to be a full sentence
previously, where 'shows' was appropriate, but I think "show" sounds
better after the colon.
> git stash clear &&
> echo 1 >subdir/subfile1 &&
> echo 2 >subdir/subfile2 &&
> @@ -1048,7 +1048,7 @@ test_expect_success 'stash push -p with pathspec shows
> no changes only once' '
> test_i18ncmp expect actual
> '
>
> -test_expect_success 'stash push with pathspec shows no changes when there
> are none' '
> +test_expect_success 'push: <pathspec> shows no changes when there are none' '
Maybe "push <pathspec>: show no changes when there are none"? "push
<pathspec>" would be the rest of the 'git stash' command, having the
colon in between them seems a bit odd.
> >foo &&
> git add foo &&
> git commit -m "tmp" &&
> @@ -1058,7 +1058,7 @@ test_expect_success 'stash push with pathspec shows no
> changes when there are no
> test_i18ncmp expect actual
> '
>
> -test_expect_success 'stash push with pathspec not in the repository errors
> out' '
> +test_expect_success 'push: <pathspec> not in the repository errors out' '
This one makes sense to me.
> >untracked &&
> test_must_fail git stash push untracked &&
> test_path_is_file untracked
> --
> 2.18.0.573.g56500d98f
>
