Re: [PATCH] t0000: modernise style

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 1, 2012, 21:54 UTC
Message-ID: <7vpqcwkmwu.fsf@alter.siamese.dyndns.org>
In-Reply-To: <8dc3b95f0d2511b1598f006df3a54c166a3b4b8c.1330637527.git.stefano.lattarini@gmail.com>

Stefano Lattarini <stefano.lattarini@gmail.com> writes:

Show 17 quoted lines

> Match the style to more modern test scripts, namely:
>
>  - Prefer tabs for indentation.
>
>  - The first line of each test has prereq, title and opening sq for the
>    script body.
>
>  - Move cleanup or initialization of data used by a test inside the test
>    itself.
>
>  - Put a newline before the closing sq for each test.
>
>  - Don't conclude the test descriptions with a full stop.
>
>  - Prefer 'test_line_count FILE COUNT' over 'test $(wc -l <FILE) = COUNT'
>
>  - Use '<<-EOF' style for here documented, so that they can be indented

s/documented,/document,/;

Show 13 quoted lines

>    as well.  Bot don't do that in case the resulting lines would be too
>    long.
>
>  - Don't redirect the output of commands to /dev/null unconditionally,
>    the git testing framework should already take care of handling test
>    verbosity transparently and uniformly.
>
> Signed-off-by: Stefano Lattarini <stefano.lattarini@gmail.com>
> ---
>
>  Not sure whether a cosmetic-only patch like this coming from an
>  "outsider" will be well-received or regarded as noise, but I guess
>  the best way to find out is to try and send it.

A pair of good rule of thumb:

 (1) who the sender is does not matter.

 (2) an area that saw high activity recently tends to repel cosmetic-only
     patches.

Show 15 quoted lines

> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
> index f4e8f43..fa2c5f4 100755
> --- a/t/t0000-basic.sh
> +++ b/t/t0000-basic.sh
> @@ -34,69 +34,69 @@ fi
>  # git init has been done in an empty repository.
>  # make sure it is empty.
>  
> -find .git/objects -type f -print >should-be-empty
> -test_expect_success \
> -    '.git/objects should be empty after git init in an empty repo.' \
> -    'cmp -s /dev/null should-be-empty'
> +test_expect_success '.git/objects should be empty after git init in an empty repo' '
> +	find .git/objects -type f -print >should-be-empty &&
> +	cmp -s /dev/null should-be-empty

It might be even better to do this

	>empty
        test_cmp empty should-be-empty

when there is a behaviour change under "-v" option?

Or be consistent with the next one and use test_line_count for zero?

I'll backburner the remainder of the patch.