Re: [PATCH 1/2] t0024: avoid losing exit status to pipes

[Junio C Hamano <gitster@pobox.com>]

Ghanshyam Thakkar <shyamthakkar001@gmail.com> writes:

> Replace pipe with redirection operator '>' to store the output
> to a temporary file after 'git archive' command since the pipe
> will swallow the command's exit code and a crash won't
> necessarily be noticed.

OK.  I think this case what the patch does is the right thing.  If
we were creating a huge tar archive and have the downstream only
consuming for a small disk footprint (e.g., "git archive | tar tf -"),
use of pipe (and loss of exit status) may be justified, but I do not
think that is the case here.


> Also refactor an existing use of '>' to avoid having a space after
> '>', according to Documentation/CodingGuidelines.

It may be just me, but I wouldn't call that "refactor", which we
often use to refer to changing the way an existing functional unit
works internally, by splitting the innard of a function into
separate helper functions, by replacing the open-coded duplicate
with calls to such helper functions, etc.  Correcting the coding
style violation is merely a "fix".

	Also fix an unwanted space after '>' redirection to match
	the style in CodingGuidelines.

In any case, the patch text looks good.

Thanks.



> Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
> ---
>  t/t0024-crlf-archive.sh | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/t/t0024-crlf-archive.sh b/t/t0024-crlf-archive.sh
> index a34de56420..fa4da7c2b3 100755
> --- a/t/t0024-crlf-archive.sh
> +++ b/t/t0024-crlf-archive.sh
> @@ -9,7 +9,7 @@ test_expect_success setup '
>  
>  	git config core.autocrlf true &&
>  
> -	printf "CRLF line ending\r\nAnd another\r\n" > sample &&
> +	printf "CRLF line ending\r\nAnd another\r\n" >sample &&
>  	git add sample &&
>  
>  	test_tick &&
> @@ -19,8 +19,8 @@ test_expect_success setup '
>  
>  test_expect_success 'tar archive' '
>  
> -	git archive --format=tar HEAD |
> -	( mkdir untarred && cd untarred && "$TAR" -xf - ) &&
> +	git archive --format=tar HEAD >test.tar &&
> +	( mkdir untarred && cd untarred && "$TAR" -xf ../test.tar ) &&
>  
>  	test_cmp sample untarred/sample