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

From: Eric Sunshine <sunshine@sunshineco.com>
Date: May 11, 2023, 19:57 UTC
Message-ID: <CAPig+cSTVbvyUOOoVrsTS8t4VSoCbVRoA=i6j0_4KZX5ZTt34Q@mail.gmail.com>
In-Reply-To: <20230511194822.1493798-1-calvinwan@google.com>

On Thu, May 11, 2023 at 3:48 PM Calvin Wan <calvinwan@google.com> wrote:

Show 18 quoted lines

> strbuf, as a generic and widely used structure across the codebase,
> should be limited as a library to only interact with primitives. Add
> documentation so future functions can appropriately be placed. Older
> functions that do not follow this boundary should eventually be moved or
> refactored.
>
> Signed-off-by: Calvin Wan <calvinwan@google.com>
> ---
> diff --git a/strbuf.h b/strbuf.h
> @@ -1,6 +1,15 @@
> +/*
> + * NOTE FOR STRBUF DEVELOPERS
> + *
> + * The objects that this API interacts with should be limited to other
> + * primitives, however, there are older functions in here that interact
> + * with non-primitive objects which should eventually be moved out or
> + * refactored.
> + */

Sorry, I meant to send a follow-up to my previous email[1] but forgot. In particular, I wanted to say that I think the second part of the above comment (the "however" and everything which follows) ought to be dropped since it will become stale once those "older functions" are finally removed; it is likely nobody will remember to update this comment. So, the above could rewritten something like this:

 /*
  * NOTE FOR STRBUF DEVELOPERS
  *
  * strbuf is a low-level primitive; as such it should interact only
  * with other low-level primitives. Do not introduce new functions
  * which interact with higher-level APIs.
  */

[1]: https://lore.kernel.org/git/CAPig+cTQg7XzORPHeD79aHEi1ggOjTPw9X02VPgxcV9uoBOBxg@mail.gmail.com/