From: Christian Couder <christian.couder@gmail.com>
Date: Wed, 28 Apr 2021 17:16:52 GMT
Subject: Re: [PATCH v2] t6030: add test for git bisect skip started with --term* arguments
Message-ID: <CAP8UFD10vLX_rF=P374BP-CeE5cxs1Rg6faxqf+m3J4MH9Vb8A@mail.gmail.com>
In-Reply-To: <CAPig+cS0yA3TP9PBi2ZtQx5UeKRbFgwXqeqB1sC6QPB7q+Lu2Q@mail.gmail.com>

On Wed, Apr 28, 2021 at 6:32 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
>
> On Wed, Apr 28, 2021 at 4:12 AM Christian Couder
> <christian.couder@gmail.com> wrote:
> > On Sun, Apr 25, 2021 at 10:06 AM Bagas Sanjaya <bagasdotme@gmail.com> wrote:
> > > +       git bisect reset &&
> > > +       git bisect start --term-new=fixed --term-old=unfixed HEAD $HASH1 &&
> > > +       HASH_SKIPPED_FROM=$(git rev-parse --verify HEAD) &&
> > > +       git bisect skip &&
> > > +       HASH_SKIPPED_TO=$(git rev-parse --verify HEAD) &&
> > > +       test $HASH_SKIPPED_FROM != $HASH_SKIPPED_TO
> >
> > It might be a bit safer and more consistent with the rest of this test
> > script to use double quotes around $HASH_SKIPPED_FROM and
> > $HASH_SKIPPED_TO, like:
> >
> >        test "$HASH_SKIPPED_FROM" != "$HASH_SKIPPED_TO"
>
> Also, is there a reason for upcasing these variable names
> (HASH_SKIPPED_FROM and HASH_SKIPPED_TO), thus making them appear to be
> globals even though they are used only in this test? More appropriate
> and less misleading names might be `skipped_from` and `skipped_to`.

In this test script many variables are called HASH1, HASH2, ... or
PARA_HASH1, PARA_HASH2, ... So in this regard HASH_SKIPPED_FROM and
HASH_SKIPPED_TO look ok to me.