Re: [PATCH 08/12] t5520: use test_cmp_rev where possible
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Oct 23, 2019, 13:53 UTC
- Message-ID
- <CAPig+cQgE=7Ac+NRaxGRjDwFv4PR1sP8FLHNJfKXJC4QmR4Dhg@mail.gmail.com>
- In-Reply-To
- <20191018185227.GA74877@generichostname>
On Fri, Oct 18, 2019 at 2:52 PM Denton Liu <liu.denton@gmail.com> wrote:
Show 19 quoted lines
> 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@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.