Re: [PATCH] config: Add documentation for writing config files

[Tanay Abhra <tanayabh@gmail.com>]

On 06/02/2014 12:37 PM, Matthieu Moy wrote:
> Tanay Abhra <tanayabh@gmail.com> writes:
> 
>> Signed-off-by: Tanay Abhra <tanayabh@gmail.com>
>> ---
>>  Documentation/technical/api-config.txt | 31 ++++++++++++++++++++++++++++++-
>>  1 file changed, 30 insertions(+), 1 deletion(-)
> 
> Even though the reason to replace a TODO with real stuff is rather
> straigthforward, the reader appriates a short commit message (ideally
> pointing to the commit introducing the TODO). My first thought reading
> the patch was "wtf, is that a patch in the middle of a series, where
> does this TODO come from" ;-).

Ok, I will send a new patch with the updated commit message. .

>> +In the end they both all call `git_config_set_multivar_in_file` which takes
>> +four parameters:
> 
> Do we really want to document this as part of the config API? i.e. does
> a normal user of the API want to know about this? My understanding is
> that git_config_set_multivar_in_file is essentially a private function,
> and then the best place to document is with comments near the function
> definition (it already has such comment).

Sorry, but I don't think so. In cache.h, the following functions have been provided
as externs,
git_config_set_in_file()
git_config_set()
git_config_set_multivar()
git_config_set_multivar_in_file()
extern int git_config_rename_section()
extern int git_config_rename_section_in_file()

Thus, they seem to be the part of the API. In most of the technical API docs I have
read there is at least a description of all parameters of the main function also,
relevant data structures if any.
Also a certain amount of redundancy about details is allowed.

A good example is api-hashmap.txt which provides detailed descriptions of each
function and data structure which was very much helpful to me.

Reverse is api-string-list.txt which was useless to me, so I had to go through
the whole code to understand how to use it.

Thanks for the review.