"Kristoffer Haugsbakk" <code@xxxxxxxxxxxxxxx> 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.
