On Mon, Nov 25, 2019 at 8:22 PM Denton Liu <liu.denton@xxxxxxxxx> wrote:
> Make the test more hash-agnostic by renaming variables from "sha1" to
> "oid" (case-insensitively). Also, replace the regex, `[0-9a-f]\{40\}`
> with `$OID_REGEX`.
>
> Signed-off-by: Denton Liu <liu.denton@xxxxxxxxx>
> ---
> diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
> @@ -6,31 +6,31 @@ test_description='git repack works correctly'
> commit_and_pack () {
> test_commit "$@" 1>&2 &&
> - SHA1=$(git pack-objects --all --unpacked --incremental .git/objects/pack/pack </dev/null) &&
> - echo pack-${SHA1}.pack
> + OID=$(git pack-objects --all --unpacked --incremental .git/objects/pack/pack </dev/null) &&
> + echo pack-${OID}.pack
> }
Meh. OID stands for "object ID". However, in this context, SHA1 is a
computed hash value -- a checksum -- not the ID of an object. So,
calling this an OID is confusing (IMHO). By the way, the uppercase
"SHA1" gives the impression that this value is global to the script,
but it is, in fact, used only in this function, so it would be clearer
to downcase the entire name, implying by convention that it is a local
variable. Taking both observations into consideration, a better name
might be "packid".
> -# we expect $packsha1 and $objsha1 to be defined
> +# we expect $packoid and $objoid to be defined
Likewise. By using "oid" in these names, you're calling these values
"pack object ID" and "object object ID", respectively, which doesn't
make much sense. Perhaps "packid" and simply "oid" would be better
names.
Same comment applies to remaining changes in this patch.
> @@ -124,7 +124,7 @@ test_expect_success 'packed obs in alternate ODB kept pack are repacked' '
> test_expect_success 'packed unreachable obs in alternate ODB are not loosened' '
> rm -f alt_objects/pack/*.keep &&
> mv .git/objects/pack/* alt_objects/pack/ &&
> - csha1=$(git rev-parse HEAD^{commit}) &&
> + coid=$(git rev-parse HEAD^{commit}) &&
This is indeed an object ID, so replacing literal "sha1" with "oid" makes sense.
