On Fri, Oct 18, 2019 at 2:52 PM Denton Liu <liu.den...@gmail.com> wrote:
> On Thu, Oct 17, 2019 at 07:41:44PM -0400, Eric Sunshine wrote:
> > On Thu, Oct 17, 2019 at 7:17 PM Denton Liu <liu.den...@gmail.com> wrote:
> > > -       test "$COPY" = "$(git rev-parse --verify me/copy)" &&
> > > +       test_cmp_rev "$COPY" me/copy &&
> >
> > This transformation doesn't look correct. COPY already holds the
> > result of a git-rev-parse invocation:
> >
> >     COPY="$(git rev-parse --verify me/copy)" &&
> >
> > so passing it to test_cmp_rev() -- which applies its own git-rev-parse
> > invocation -- doesn't make sense.
>
> So after grokking the test case, it seems like the the transformation is
> indeed correct. Maybe we can replace the last line with
>
>         test_cmp_rev copy me/copy
>
> but I think I'll leave it unless you have any strong opinions.

For some reason, I had it in my mind that test_cmp_rev() was primarily
meant for comparing _named_ revisions, but of course there is nothing
about the function which even suggests that that is its intended
use-case. In retrospect, using it to compare an OID against a named
revision is a sensible use-case too, so I withdraw the objection.

Reply via email to