Re: [PATCH 2/8] t5512: generate references with generate_references()

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 25, 2020, 06:41 UTC
Message-ID: <xmqqmu85x7fo.fsf@gitster.c.googlers.com>
In-Reply-To: <674de50db28a50554d7af6e5c869c427d06f78aa.1585115341.git.liu.denton@gmail.com>

Denton Liu <liu.denton@gmail.com> writes:

> +generate_references () {
> +	for i

Is it just me who thinks variables used in iteration should be called i, j, etc. only when they are integers?

Show 20 quoted lines

> +	do
> +		oid=$(git rev-parse "$i") || return 1
> +		printf '%s\t%s\n' "$oid" "$i"
> +	done
> +}
> +
>  test_expect_success setup '
>  	>file &&
>  	git add file &&
> @@ -43,34 +51,19 @@ test_expect_success 'ls-remote self' '
>  '
>  
>  test_expect_success 'ls-remote --sort="version:refname" --tags self' '
> -	cat >expect <<-EOF &&
> -	$(git rev-parse mark)	refs/tags/mark
> -	$(git rev-parse mark1.1)	refs/tags/mark1.1
> -	$(git rev-parse mark1.2)	refs/tags/mark1.2
> -	$(git rev-parse mark1.10)	refs/tags/mark1.10
> -	EOF
> +	generate_references refs/tags/mark refs/tags/mark1.1 refs/tags/mark1.2 refs/tags/mark1.10 >expect &&

Hmph, can we avoid overlong lines like this one?

	generate_references >expect <<-EOF
	refs/tags/mark
	refs/tags/mark1.1
	refs/tags/mark1.2
	refs/tags/mark1.10
	EOF

i.e. teaching the helper function to read from its standard input stream, may make it more readable (i.e. it is more obvious what the order of expected output lines are, as you are listing them one by one on a line of its own).