On 06/02/2014 12:37 PM, Matthieu Moy wrote: > Tanay Abhra <tanayabh@xxxxxxxxx> writes: > >> Signed-off-by: Tanay Abhra <tanayabh@xxxxxxxxx> >> --- >> 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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html