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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to