Re: [PATCH v1] builtin/mktree: remove USE_THE_REPOSITORY_VARIABLE

[Patrick Steinhardt <ps@pks.im>]

On Thu, Mar 12, 2026 at 02:17:03AM +0800, Tian Yuchen wrote:
> The 'cmd_mktree' already receives a 'struct repository *repo' but was
> previously marked as UNUSED.
> 
> Pass the 'repo' down the 'mktree-line()' and 'write_tree()'.

I guess s/the/to/? Also, it's `mktree_line()`, not `mktree-line()`.

One thing that commit messages should also explain is why a certain
refactoring is safe to do. That is, can `repo` ever be `NULL`? For that
you have to look at "git.c" and figure out whether or not the command
requires a repository to exist.

> The 'oid_to_hex' function appears to use 'the_hash_algo' internally.
> Seems that it also implicitly relying on global state. Is there
> anything we should be aware of?

`oid_to_hex()` falls back to using `the_hash_algo` in case the object ID
you have doesn't have a proper hash specified. So this depends on how
exactly you construct the object IDs: if you parse them with a proper
hash algorithm, then you're fine.

> I've always been unsure about who to CC on domain-specific patches,
> so I've only been sending them to Junio and the mailing list. Could
> this be why my previous patch for a global variable refactor didn't
> receive any review feedback? Here is the link:

It's typically fine to just send to the mailing list, so you wouldn't
even Cc Junio. Sometimes it's just a matter of capacity, and it's fine
to eventually send a ping after a week or two have passed without any
feedback.

The patch itself looks good to me, thanks!

Patrick