Re: [PATCH 2/3] t4216: fix broken '&&'-chain

[Jeff King <peff@peff.net>]

On Tue, Jun 30, 2020 at 01:50:22PM -0400, Eric Sunshine wrote:

> > diff --git a/t/t4216-log-bloom.sh b/t/t4216-log-bloom.sh
> > @@ -53,7 +53,7 @@ sane_unset GIT_TRACE2_PERF_BRIEF
> >  sane_unset GIT_TRACE2_CONFIG_PARAMS
> 
> Not related to this patch, but 'sane_unset' is pointless outside of a
> test since there is no &&-chain to maintain. Plain 'unset' would work
> just as well and be less misleading.

We do this in lots of other places, too. Try:

  git grep sane_unset | grep -v '&&'

Though seeing the variables they cover, I think many of them may come
from the same few authors. :)

I wonder if it is worth keeping these as sane_unset, though. Your
comment is based on the knowledge that the "sane" part of the function
is ignoring the return value. But we could conceivably have other
portability fixes (it's not impossible that a shell wants one "unset"
per variable, for example), in which case we'd want it in more places.

That's hypothetical, of course, but saying "just use our portable unset
wrapper that behaves sanely" seems like exactly what these call sites
want, now and in a hypothetical future.

-Peff