On Wed, May 18, 2016 at 1:32 PM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: > 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. On reflection, I agree with your points and will include the change in the re-roll. 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