Re: [PATCH 3/4] rev-parse test: use test_cmp instead of "test" builtin

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

 



Junio C Hamano wrote:

> After applying this patch and running "git show | grep test_cmp_rev_output",
> I notice that the second is always "git rev-parse <something>".  Do
> we still need to eval these, or would it be sufficient to do
>
>         test_cmp_rev_output () {
>                 git rev-parse --verify "$1" >expect &&
>                 git rev-parse --verify "$2" >actual &&
>                 test_cmp expect actual
>         }
>
> here, and make users like so:
>
>	-	test_cmp_rev_output tags/start "git rev-parse start^0"
>	+	test_cmp_rev_output tags/start start^0
>
> Am I missing something?

I was tempted to use test_cmp_rev, which would have the same effect.
The original test checked the output of "git rev-parse start^0", while
the test as modified above checks the output of "git rev-parse
--verify start^0".

I slightly prefer the version without --verify because "git rev-parse
--verify" is well exercised elsewhere in the testsuite (but then, so
is rev-parse without --verify, so it's not a big deal).

Abbreviating as follows

	foo () {
		git rev-parse --verify "$1" >expect &&
		git rev-parse "$2" >actual &&
		test_cmp expect actual
	}

would also be fine with me.  All it would take is a good replacement
for the placeholder "foo".  The added flexibility of "compare a rev
to the output of an arbitrary command" doesn't get used.

Jonathan
--
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]