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

From: Patrick Steinhardt <ps@pks.im>
Date: Jan 19, 2024, 06:18 UTC
Message-ID: <ZaoUOPsze7rhtT2M@tanuki>
In-Reply-To: <32e5ec7d866ff8fd26554b325812c6e19cb65126.1705267839.git.code@khaugsbakk.name>

On Sun, Jan 14, 2024 at 10:43:18PM +0100, Kristoffer Haugsbakk wrote:

Show 5 quoted lines

> 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