Re: [PATCH v2 2/5] t1500: test_rev_parse: facilitate future test enhancements

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

 



On Wed, May 18, 2016 at 12:38 PM, SZEDER Gábor <szeder@xxxxxxxxxx> wrote:
> Quoting Eric Sunshine <sunshine@xxxxxxxxxxxxxx>:
>> +       for o in is-bare-repository \
>> +                is-inside-git-dir \
>> +                is-inside-work-tree \
>> +                show-prefix \
>> +                git-dir
>> +       do
>> +               test $# -eq 0 && break
>> +               expect="$1"
>> +               test_expect_success "$name: $o" '
>> +                       echo "$expect" >expect &&
>> +                       git rev-parse --$o >actual &&
>
> I think that "--$o" looks really weird, but that's subjective, of course.
>
> However, the idea popped up in an other thread[1] that we might want
> something like 'git rev-parse --absolute-path --git-dir', which wouldn't
> really work with '--$o'.
>
> Even if we don't go that route, perhaps it would be better to list the
> options to be tested including their doubledash prefix.

As this series is only about modernizing t1500, I'd prefer to keep the
conversion faithful to the original which titles each test "$name:
is-bare-repository", "$name: is-inside-git-dir", etc., and the current
approach does so without additional complexity.

I have no objection to upgrading the for-loop items to include the
leading dashes or updating the logic to support --absolute-path, but
such changes are outside the scope of this series and can easily be
built atop it. Also, due to severe time constraints, I'd rather not
re-roll only for a superficial and subjective change such as adding
leading dashes to for-loop items.

Thanks.
--
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]