Subject: Re: [PATCH v2 03/10] midx-write: use `revs->repo` inside `read_refs_snapshot`

From: Taylor Blau <[email-masked]>
Date: Wed, 20 Nov 2024 14:46:55 -0500

On Wed, Nov 20, 2024 at 02:26:23PM +0000, Richard Kerry wrote:
>
> >> The `read_refs_snapshot` uses the `parse_oid_hex` function which
> >> internally uses global variables. Let's instead use
> >
> >Nit: s/variables/variable
>
> No, that's fine.
> It's plural, so ends with 's'.
> Unless it should be "uses a global variable"

The global variable in question here is just "the_hash_algo", so I think
shejialuo's suggestion to use "variable" is correct, but it would need
to be "uses a global variable" instead of "uses global variable"
(without the article).

But I think we're being unnecessarily vague here, and could instead say:

    The function `read_refs_snapshot()` uses `parse_oid_hex()`, which
    relies on the global `the_hash_algo` variable. Let's instead use
    [...]

> >> Also, while here, fix a missing newline after the functions definition.
> >>
> >
> >Nit: s/functions/function
>
> Maybe.
> But it could be "the function's definition" as it could be seen as possessive.

It should be "function's definition", as the possessive is the correct
form.

Thanks,
Taylor