Subject: Re: [PATCH 1/8] sha1-array: provide oid_array_filter

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Wed, 26 Sep 2018 19:49:57 +0200

On Wed, Sep 26 2018, Junio C Hamano wrote:

> Jeff King <[email-masked]> writes:
>
>> On Tue, Sep 25, 2018 at 12:26:44PM -0700, Stefan Beller wrote:
>>
>>> On Sat, Sep 22, 2018 at 5:58 AM Ævar Arnfjörð Bjarmason
>>> <[email-masked]> wrote:
>>> >
>>> >
>>> > On Fri, Sep 21 2018, Stefan Beller wrote:
>>> >
>>> > > +/*
>>> > > + * Apply want to each entry in array, retaining only the entries for
>>> > > + * which the function returns true.  Preserve the order of the entries
>>> > > + * that are retained.
>>> > > + */
>>> > > +void oid_array_filter(struct oid_array *array,
>>> > > +                   for_each_oid_fn want,
>>> > > +                   void *cbdata);
>>> > > +
>>> > >  #endif /* SHA1_ARRAY_H */
>>> >
>>> > The code LGTM, but this comment should instead be an update to the API
>>> > docs, see my recent 5cc044e025 ("get_short_oid: sort ambiguous objects
>>> > by type, then SHA-1", 2018-05-10) for an addition of a new function to
>>> > this API where I added some new docs.
>>>
>>> ok will fix for consistency (this whole API is there).
>>>
>>> Longer term (I thought) we were trying to migrate API docs
>>> to headers instead?
>>
>> Yes, please. I think it prevents exactly this sort of confusion. :)
>
> CodingGuidelines or SubmittingPatches update, perhaps?
>
>  Documentation/CodingGuidelines | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
> index 48aa4edfbd..b54684e807 100644
> --- a/Documentation/CodingGuidelines
> +++ b/Documentation/CodingGuidelines
> @@ -358,7 +358,11 @@ For C programs:
>     string_list for sorted string lists, a hash map (mapping struct
>     objects) named "struct decorate", amongst other things.
>
> - - When you come up with an API, document it.
> + - When you come up with an API, document it.  It used to be
> +   encouraged to do so in Documentation/technical/, and the birds-eye
> +   level overview may still be more suitable there, but detailed
> +   function-by-function level of documentation is done by comments in
> +   corresponding .h files these days.
>
>   - The first #include in C files, except in platform specific compat/
>     implementations, must be either "git-compat-util.h", "cache.h" or

Thanks. I had not looked at this closely and was under the false
impression that it was going in the other direction. Good to have it
clarified.