From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 19 Jan 2024 18:36:05 GMT
Subject: Re: [PATCH v2 3/4] config: factor out global config file retrieval
Message-ID: <xmqq34utkw6i.fsf@gitster.g>
In-Reply-To: <7f0864ad-c846-42a6-8ddc-85d6be58a4ee@app.fastmail.com>

"Kristoffer Haugsbakk" <code@khaugsbakk.name> writes:

> On Fri, Jan 19, 2024, at 07:18, Patrick Steinhardt wrote:
>> But second, I think that the new function you introduce here has the
>> same issue as the old function that you refactored in the preceding
>> patch: `git_config_global()` isn't very descriptive, and it is also
>> inconsistent the new `git_config_global_paths()`. I'd propose to name
>> the new function something like `git_config_global_preferred_path()` or
>> `git_config_global_path()`.
>
> The choice of `git_config_global` is mostly motivated by it working the
> same way as `git_config_system`:
>
> ```
> given_config_source.file = git_system_config();
> […]
> given_config_source.file = git_global_config();
> ```

I shared the above understanding with you, so I didn't find the name
"not very descriptive" during my review.  If only we had two more
functions that can replace our uses of repo_git_path(r, "config")
and repo_git_path(r, "config.worktree") [*] in the code, to obtain
the path to the repository local and worktree local configuration
files, the convention may have been more obvious.

    Side note: the worktree specific one is messier; there are code
    paths that use "%s/config.worktree" on gitdir as well---if we
    were to introduce helpers, we should catch and convert them, too.

> Your suggestion makes sense. But should `git_system_config` be renamed as
> well?

I do not mind including "path" in the names of these functions, but
I do agree that such renaming should be done consistently across the
family of functions (which we currently have only two members, but
still).

Thanks.