From: Taylor Blau <me@ttaylorr.com>
Date: Sat, 16 Mar 2024 01:34:23 GMT
Subject: Re: [PATCH v2 09/22] t/t4*: avoid redundant uses of cat
Message-ID: <ZfT3H7IiewuhAA7M@nand.local>
In-Reply-To: <20240315194620.10713-10-dev+git@drbeat.li>

On Fri, Mar 15, 2024 at 08:46:06PM +0100, Beat Bolli wrote:
> diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh
> index 12ac43687366..578641467753 100755
> --- a/t/t4301-merge-tree-write-tree.sh
> +++ b/t/t4301-merge-tree-write-tree.sh
> @@ -313,7 +313,7 @@ test_expect_success 'rename/add handling' '
>  		# First, check that the bar that appears at stage 3 does not
>  		# correspond to an individual blob anywhere in history
>  		#
> -		hash=$(cat out | tr "\0" "\n" | head -n 3 | grep 3.bar | cut -f 2 -d " ") &&
> +		hash=$(tr "\0" "\n" <out | head -n 3 | grep 3.bar | cut -f 2 -d " ") &&
>  		git rev-list --objects --all >all_blobs &&
>  		! grep $hash all_blobs &&

This and the two similar transformations below it look good to me. This
obviously isn't the fault of your patch (nor should it necessarily be
its aim to fix), but I wonder if it would be worthwhile to extract the

    tr "\0" "\n" <out | head -n 3 | grep 3.bar | cut -f 2 -d " "

pattern into a helper function, since it's used in a few places in this
test script.

That's just a suggestion, and shouldn't hold up this patch/series. Maybe
just some #leftoverbits :-).

Thanks,
Taylor