Subject: Re: [PATCH v4 1/7] strbuf: clarify API boundary

From: Junio C Hamano <[email-masked]>
Date: Wed, 10 May 2023 15:51:49 -0700

Eric Sunshine <[email-masked]> writes:

> On Mon, May 8, 2023 at 1:05 PM Calvin Wan <[email-masked]> wrote:
>> strbuf, as a generic and widely used structure across the codebase,
>> should be limited as a libary to only interact with primitives. Add
>
> s/libary/library/
>
>> documentation so future functions can be appropriately be placed. Older
>
> Too many "be"'s.
>
>> functions that do not follow this boundary should eventually be moved or
>> refactored.
>>
>> Signed-off-by: Calvin Wan <[email-masked]>
>> ---
>> diff --git a/strbuf.h b/strbuf.h
>> @@ -5,7 +5,11 @@ struct string_list;
>>  /**
>>   * strbuf's are meant to be used with all the usual C string and memory
>> - * APIs. Given that the length of the buffer is known, it's often better to
>> + * APIs. The objects that this API interacts with in this file should be
>> + * limited to other primitives, however, there are older functions in here
>> + * that should eventually be moved out or refactored.
>> + *
>> + * Given that the length of the buffer is known, it's often better to
>>   * use the mem* functions than a str* one (memchr vs. strchr e.g.).
>>   * Though, one has to be careful about the fact that str* functions often
>>   * stop on NULs and that strbufs may have embedded NULs.
>
> The new text is administrative in nature, aimed at people who will be
> modifying strbuf itself. As such, it is unclear why it is being
> inserted into documentation aimed at _consumers_ of the strbuf API.
> Moreover, with it buried in existing API documentation like this, I
> fear that those at whom it is aimed will almost certainly overlook it.
>
> To increase the likelihood that the target audience will indeed read
> the new text, I'd suggest placing it in its own comment block very
> near the top of the file, possibly prefixed with a loud "NOTE FOR
> STRBUF DEVELOPERS" or some such. Further, as the new text is aimed at
> strbuf developers, not strbuf consumers, it would make more sense to
> use a plain /*...*/ comment block rather than a /**...*/ block.

All look good suggestions to make.

If there is nothing else outstanding, let's see a small and
hopefully final reroll so that the topic can be merged to 'next'
soonish.

Thanks.