Re: [PATCH v2 3/4] config: factor out global config file retrieval

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 1220 bytes --]

On Sun, Jan 14, 2024 at 10:43:18PM +0100, Kristoffer Haugsbakk wrote:
> Factor out code that retrieves the global config file so that we can use
> it in `gc.c` as well.
> 
> Use the old name from the previous commit since this function acts
> functionally the same as `git_system_config` but for “global”.

I was briefly wondering whether we also want to give this new function a
more descriptive name. For one, calling it `git_system_config()` which
we have just removed in the preceding set may easily lead to confusion
for any in-flight patch series because the parameters now got dropped
(or at least it looks like that).

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()`.

Sorry for not mentioning this in my first review round. Also, it's only
a minor concern, nothing that needs to block this series if either you
or others disagree with my opinion.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]