On Friday, January 19, 2024 1:36 PM, Junio C Hamano wrote: >"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). Is this going to impact the libification effort? I am just curious what other on the team think. --Randall
