On Fri, Oct 18, 2019 at 2:52 PM Denton Liu <liu.denton@xxxxxxxxx> 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.denton@xxxxxxxxx> 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.
