Re: [PATCH v2 1/4] t7401: modernize style

From: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
Date: Aug 13, 2020, 08:06 UTC
Message-ID: <c70f7bb5d1289f5318da7b99e41cf5828a451174.camel@gmail.com>
In-Reply-To: <20200812192737.13971-2-shouryashukla.oo@gmail.com>

On Thu, 2020-08-13 at 00:57 +0530, Shourya Shukla wrote:

Show 7 quoted lines

>
> [...]
>
> Using a Git command in the upstream of a pipe might result in us
> losing its exit code. So, convert such usages so that they write to
> a file and read from them.
>

While that is a good enough reason to avoid using pipes in places where we look for the exit code of a command like within test_expect_success, I'm not sure if that reason holds for the places that the patch changes.

Show 20 quoted lines

> [...]
>
> diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-
> summary.sh
> index 9bc841d085..8ee78bcb69 100755
> --- a/t/t7401-submodule-summary.sh
> +++ b/t/t7401-submodule-summary.sh
> @@ -16,12 +16,13 @@ add_file () {
>  	owd=$(pwd)
>  	cd "$sm"
>  	for name; do
> -		echo "$name" > "$name" &&
> +		echo "$name" >"$name" &&
>  		git add "$name" &&
>  		test_tick &&
>  		git commit -m "Add $name"
>  	done >/dev/null
> -	git rev-parse --verify HEAD | cut -c1-7
> +	git rev-parse --verify HEAD >out &&
> +	cut -c1-7 out

In any case, I believe we can avoid the 'cut' altogether in both places by doing something like this instead:

   git rev-parse --short=7 HEAD

My quick check shows the test script is happy with this change.

Show 13 quoted lines

>  	cd "$owd"
>  }
>  commit_file () {
> @@ -125,7 +126,8 @@ commit_file sm1 &&
>  head3=$(
>  	cd sm1 &&
>  	git reset --hard HEAD~2 >/dev/null &&
> -	git rev-parse --verify HEAD | cut -c1-7
> +	git rev-parse --verify HEAD >out &&
> +	cut -c1-7 out
>  )
>  
>  test_expect_success 'modified submodule(backward)' "

-- 
Sivaraam