Re: [PATCH-w 101/105] t6300 (for-each-ref): modernize style

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 1, 2012, 02:13 UTC
Message-ID: <7v62epqd9a.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1330566326-26075-7-git-send-email-tmgrennan@gmail.com>

Tom Grennan <tmgrennan@gmail.com> writes:

> +if ! test -r test-lib.sh ; then
> +	(cd ${0%/*} && ./${0##*/} $@)
> +	exit $?
> +fi

Not very nice; why is this an improvement?

Show 5 quoted lines

>  . ./test-lib.sh
>  . "$TEST_DIRECTORY"/lib-gpg.sh
>  
> +quiet () { "$@" >/dev/null; }
> +silent () { "$@" >/dev/null 2>&1; }

Not nice, either.

Show 12 quoted lines

>  # Mon Jul 3 15:18:43 2006 +0000
>  datestamp=1151939923
>  setdate_and_increment () {
> @@ -22,9 +30,9 @@ test_expect_success 'Create sample commit with known timestamp' '
>  	setdate_and_increment &&
>  	echo "Using $datestamp" > one &&
>  	git add one &&
> -	git commit -m "Initial" &&
> +	git commit -q -m "Initial" &&
>  	setdate_and_increment &&
> -	git tag -a -m "Tagging at $datestamp" testtag
> +	quiet git tag -a -m "Tagging at $datestamp" testtag

Why? Why? Why?

	cd t && sh ./t1234-frotz.sh

would be silent enough and suppressing the output from the commands like this patch does makes it _harder_ for people to debug their changes to the script with

	sh ./t1234-frotz.sh -v

Most of the change in this patch does nothing to do with "modernize style".