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

From: Patrick Steinhardt <ps@pks.im>
Date: Mar 12, 2026, 06:55 UTC
Message-ID: <abJjYNq_sxeH8yLQ@pks.im>
In-Reply-To: <20260311181704.958509-1-cat@malon.dev>

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