Subject: Re: [PATCHv2 2/6] submodule test invocation: only pass additional arguments

From: Junio C Hamano <[email-masked]>
Date: Tue, 23 May 2017 15:26:55 +0900

Stefan Beller <[email-masked]> writes:

> diff --git a/t/t2013-checkout-submodule.sh b/t/t2013-checkout-submodule.sh
> index e8f70b806f..2672f104cf 100755
> --- a/t/t2013-checkout-submodule.sh
> +++ b/t/t2013-checkout-submodule.sh
> @@ -65,9 +65,9 @@ test_expect_success '"checkout <submodule>" honors submodule.*.ignore from .git/
>  
>  KNOWN_FAILURE_DIRECTORY_SUBMODULE_CONFLICTS=1
>  KNOWN_FAILURE_SUBMODULE_RECURSIVE_NESTED=1
> -test_submodule_switch_recursing "git checkout --recurse-submodules"
> +test_submodule_switch_recursing "checkout"
>  
> -test_submodule_forced_switch_recursing "git checkout -f --recurse-submodules"
> +test_submodule_forced_switch_recursing "checkout -f"
>  
>  test_submodule_switch "git checkout"

Doesn't the above look crazy to you?  

It is hostile to other people (and those who need to make merges)
who have to work with test_submodule_switch_recursing that older one
used to take the full command but its definition suddenly changes so
that the caller now must omit the leading "git".  Even worse,
another helper with a similar-sounding name, test_submodule_switch,
still must be called with the leading "git".

The same comment applies to the one we can see below.

> diff --git a/t/t7112-reset-submodule.sh b/t/t7112-reset-submodule.sh
> index f86ccdf215..a000304221 100755
> --- a/t/t7112-reset-submodule.sh
> +++ b/t/t7112-reset-submodule.sh
> @@ -9,9 +9,9 @@ KNOWN_FAILURE_SUBMODULE_RECURSIVE_NESTED=1
>  KNOWN_FAILURE_DIRECTORY_SUBMODULE_CONFLICTS=1
>  KNOWN_FAILURE_SUBMODULE_OVERWRITE_IGNORED_UNTRACKED=1
>  
> -test_submodule_switch_recursing "git reset --recurse-submodules --keep"
> +test_submodule_switch_recursing "reset --keep"
>  
> -test_submodule_forced_switch_recursing "git reset --hard --recurse-submodules"
> +test_submodule_forced_switch_recursing "reset --hard"
>  
>  test_submodule_switch "git reset --keep"