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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Feb 29, 2012 at 06:13:53PM -0800, Junio C Hamano wrote:
>Tom Grennan <tmgrennan@xxxxxxxxx> writes:
>
>> +if ! test -r test-lib.sh ; then
>> +	(cd ${0%/*} && ./${0##*/} $@)
>> +	exit $?
>> +fi
>
>Not very nice; why is this an improvement?

I liked being able to,
	t/t1234-frotz.sh [-q|-v]
in addition to,
	cd t && sh ./t1234-frotz.sh

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

"Not nice" b/c they're on one line?
I like simple things compressed so I quickly glance pass them.
Sort of forest through the trees.

>>  # 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

Hmm, I found the noise distracting.  For example, w/o redirecting
stderr, most of the test_must_fail puke as expected.  If it's expected,
why show it?  BTW since it's a different stream, to get the error near
the exec log one has to:

	sh ./t1234-frotz.sh -v |& less

Otherwise it's challenging to separate the expected vs unexpected failures.

Most of the "quiet's"  are with git-tag and git-branch.  I'd prefer
--quiet and --silent options to these commands like git-init, git-commit, etc.

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

It's trivial to remove these "quiet" and "silent", but to me that's the
only value added by these patches.  More seriously, the remaining
modernization still seems much larger than its value.

-- 
TomG
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]