On 2020-07-23 at 04:51:32, Eric Sunshine wrote:
> On Wed, Jul 22, 2020 at 9:11 PM brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
> > In some tests, we have data files which are written with a particular
> > hash algorithm. Instead of keeping two copies of the test files, we can
> > keep one, and translate the value on the fly.
> >
> > In order to do so, we'll need to read both the source algorithm and the
> > current algorithm, so add an optional flag to the test_oid helper that
> > lets us read look up a value for a specified hash algorithm. This should
> 
> Readers trip over confusing grammar: "lets us read look up a value"
> 
> > not cause any conflicts with existing tests, since key arguments to
> > test_oid are allowed to contains only shell identifier characters.
> >
> > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> > ---
> > diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
> > @@ -928,6 +928,17 @@ test_expect_success 'test_oid can look up data for SHA-256' '
> > +test_expect_success 'test_oid can look up data a specified algorithm' '
> 
> Readers trip over confusing grammar: "can look up data a specified".

Yeah, these are both typos.  Will fix.

> > diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> > @@ -1468,7 +1468,17 @@ test_oid_cache () {
> >  # Look up a per-hash value based on a key ($1).  The value must have been loaded
> >  # by test_oid_init or test_oid_cache.
> >  test_oid () {
> 
> Should the function documentation be updated to talk about the new
> --hash option?

Sure.

> > +       case "$1" in
> > +               --hash=*)
> > +                       algo="${1#--hash=}" &&
> 
> Bikeshedding: I wonder if this should be named "--algo"?

We already have a use of this in t5515; I appear to have accidentally
sent it in an earlier series.  I can, of course, change it, but I don't
feel strongly that one or the other is better.  If you do feel strongly,
or you think it's confusing, I'm happy to change it.
-- 
brian m. carlson: Houston, Texas, US