Re: [PATCH 5/6] t1500: avoid setting environment variables outside of tests

From: Jeff King <peff@peff.net>
Date: May 10, 2016, 20:41 UTC
Message-ID: <20160510204159.GA19958@sigill.intra.peff.net>
In-Reply-To: <CAPig+cQ0jEt10p--K8jNrFBi21PkmFXuHpUPewuOWrpx+3Ma7g@mail.gmail.com>

On Tue, May 10, 2016 at 03:59:42PM -0400, Eric Sunshine wrote:

Show 22 quoted lines

> On Tue, May 10, 2016 at 3:48 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> > Actually, I think we can have improved encapsulation and maintain
> > readability like this:
> >
> >     case "$1" in
> >     ...
> >     -g) env="$2"; shift;  shift ;;
> >     ...
> >     esac
> >
> >     ...
> >     test_expect_success "..." '
> >         if test -n "$env"
> >         do
> >             test_when_finished "unset GIT_DIR"
> >             GIT_DIR="$env"
> >             export GIT_DIR
> >         fi
> >         ...
> >     '
> 
> At this point, I'd also rename 'env' to 'gitdir' to be more meaningful.

Yeah, I like this even better. As much as I enjoy eval tricks, I think this case is more readable with the condition written out.

-Peff