Re: [PATCH 02/22] sequencer: mark repository argument as unused

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Aug 30, 2023, 13:35 UTC
Message-ID: <48d7763b-2661-48af-8e64-00178de612ed@gmail.com>
In-Reply-To: <20230829233252.GA225495@coredump.intra.peff.net>

On 30/08/2023 00:32, Jeff King wrote:

Show 15 quoted lines

> On Tue, Aug 29, 2023 at 04:55:30PM +0100, Phillip Wood wrote:
> 
>>> We could change the path helper to use REPO_GIT_PATH_FUNC(), but doing
>>> so piecemeal is not great. There are 41 uses of GIT_PATH_FUNC() in
>>> sequencer.c,
>>
>> Wow, I knew there were quite a few but I hadn't realized there were that
>> many. Changing them all to take a struct repository will be a big change and
>> will make struct repo_cache_path a lot larger.
> 
> Yeah. One of the things that gave me pause is that some of them may need
> to be renamed, as well. Most of the existing ones are static local
> within sequencer.c, so names like git_path_head_file() are OK. But
> REPO_GIT_PATH_FUNC() requires a pointer in the global repo_cache_path,
> so the names need to be appropriate for a global view.

It's definitely worth leaving that for a separate patch series. A lot of the paths start rebase_path_... (I think it is only the paths used by cherry-pick/revert that start git_path_...) so we'd need a separate macro unless we renamed them. Having a different prefix is probably a good thing for making them global functions but it still leaves the problem of renaming the ones like git_path_head_file().

Best Wishes

Phillip