Re: [PATCH v5 18/39] t8002: make hash size independent

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jul 29, 2020, 02:24 UTC
Message-ID: <CAPig+cTw0XXNcKp5vVLhPtV5jH9d9gwfuYGuLqpSXsGmpn_fqw@mail.gmail.com>
In-Reply-To: <20200728233446.3066485-19-sandals@crustytoothpaste.net>

On Tue, Jul 28, 2020 at 7:35 PM brian m. carlson <sandals@crustytoothpaste.net> wrote:

Show 10 quoted lines

> Compute the length of an object ID instead of hard-coding 40-based
> values.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
> diff --git a/t/t8002-blame.sh b/t/t8002-blame.sh
> @@ -6,6 +6,10 @@ test_description='git blame'
> +test_expect_success 'setup' '
> +       hexsz=$(test_oid hexsz)
> +'

In the previous version of this series, the "setup" test invoked test_oid_init() as its very first step. This version doesn't. As a reviewer, I was caught off-guard by this unexpected and unexplained difference between versions. The script works fine without test_oid_init() anyhow since test-lib.sh invokes test_oid_init(), so the test_oid_init() call introduced here by the previous version was redundant.

Some of the patches in this series add test_oid_init() calls to their "setup" tests, while others don't, which makes for a somewhat confusing impression as one reads the series. In general, it would be nice for the patches to paint a consistent picture (i.e either uniformly employ test_oid_init() or don't), however, I would not want to see a re-roll just for that. Also, since the final patch of the series ends up removing all those test_oid_init() calls anyhow, it's all straightened out in the end.