Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
> Have the "git_configset_get_value_multi()" function and its siblings
> return an "int" and populate a "**dest" parameter like every other
> git_configset_get_*()" in the API.
>
> As we'll take advantage of in subsequent commits, this fixes a blind
> spot in the API where it wasn't possible to tell whether a list was
> empty from whether a config key existed. For now we don't make use of
> those new return values, but faithfully convert existing API users.
I think the commit message is fine as-is, but perhaps you intended to
include this paragraph from v4 [1]?
A logical follow-up to this would be to change the various "*_get_*()"
functions to ferry the git_configset_get_value() return value to their
own callers, e.g. git_configset_get_int() returns "1" rather than
ferrying up the "-1" that "git_configset_get_value()" might return,
but that's not being done in this series
Which is nice, but the commit message reads fine without it too.
1. https://lore.kernel.org/git/patch-v4-5.9-23449ff2c4e-20230202T131155Z-avarab@xxxxxxxxx/
>
> Most of this is straightforward, commentary on cases that stand out:
>
> - To ensure that we'll properly use the return values of this function
> in the future we're using the "RESULT_MUST_BE_USED" macro introduced
> in [1].
>
> As git_die_config() now has to handle this return value let's have
> it BUG() if it can't find the config entry. As tested for in a
> preceding commit we can rely on getting the config list in
> git_die_config().
>
> - The loops after getting the "list" value in "builtin/gc.c" could
> also make use of "unsorted_string_list_has_string()" instead of using
> that loop, but let's leave that for now.
>
> - In "versioncmp.c" we now use the return value of the functions,
> instead of checking if the lists are still non-NULL.
>
> 1. 1e8697b5c4e (submodule--helper: check repo{_submodule,}_init()
> return values, 2022-09-01),
