Re: [PATCH 1/2] config: document git config getter return value.

On Thu, Aug 02, 2018 at 08:29:48PM -0700, Jonathan Nieder wrote:

(cc-ing peff, config whiz)

Actually, this is all about the configset caching layer, which I never
really worked on. This is mostly from Tanay Abhra [off-list ref],
who was a GSoC student mentored by Matthieu Moy [off-list ref].

That said...

quoted

+
+/*
+ * These functions return 1 if not found, and 0 if found, leaving the found
+ * value in the 'dest' pointer.
+ */
+extern int git_configset_add_file(struct config_set *cs, const char *filename);

This function doesn't take a 'dest' argument.  Is the comment meant to
apply to it?  If not, can the comment go below it?

This is returning the value of git_config_from_file(), which is 0/-1. I
think it should be left where it is in the original, and not part of
this block of functions.

Other than that, the patch seems sane to me (I think the 0/1 return
value from these "get" functions is unnecessarily inconsistent with the
rest of Git, but changing it would be a pain. Documenting it is at least
a step forward).

-Peff