On Fri, Jan 19, 2024 at 08:40:51AM +0100, Kristoffer Haugsbakk wrote: > 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(); > ``` > > (The extra logic imposed by XDG for “global” is implied by `man git > config`. I don’t know what the guidelines are for spelling that out or not > in the internal functions.) > > Your suggestion makes sense. But should `git_system_config` be renamed as > well? Yeah, you're right that `git_system_config()` is bad in the same way. In fact I think it's worse here because we have both `git_config_system()` and `git_system_config()`, which has certainly confused me multiple times in the past. So I'd be happy to see it renamed, as well, either now or in a follow-up patch series. But as I said, I don't think it's a prereq for this patch series to land and others may have differing opinions. So please, go ahead as you deem fit (or wait for other opinions). Patrick
Attachment:
signature.asc
Description: PGP signature
