Re: [PATCH v1] builtin/mktree: remove USE_THE_REPOSITORY_VARIABLE
- From
Tian Yuchen <cat@malon.dev>
- Date
- Mar 12, 2026, 16:21 UTC
- Message-ID
- <af2c4ae3-c273-40ba-bbca-cbbf687b1b91@malon.dev>
- In-Reply-To
- <abJjYNq_sxeH8yLQ@pks.im>
Hi Patrick,
Thanks for the review!
On 3/12/26 14:55, Patrick Steinhardt wrote:
> I guess s/the/to/? Also, it's `mktree_line()`, not `mktree-line()`.
Thank you for pointing that out. I'll correct it right away.
> One thing that commit messages should also explain is why a certain > refactoring is safe to do.
I'll add it.
> 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.
I checked git.c and found that there is:
{ "mktree", cmd_mktree, RUN_SETUP }in commands[]. If my understanding is correct, before cmd_mktree is called, setup_git_directory() must have been fully executed. In that case, if the current directory isn't a valid repository (NULL), it should have already exited at an earlier stage, right?
> `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 see. That's pretty much what I had in mind.
> 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.
Oh, I see. I thought “repeatedly bringing up a patch no one cares about” would be considered kinda *impolite*. Now I understand. Thank you.
Regards,
Yuchen