| Message ID | 20230511194822.1493798-1-calvinwan@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | strbuf cleanups | expand |
On Thu, May 11, 2023 at 3:48 PM Calvin Wan <calvinwan@google.com> wrote: > 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/
> 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. > */ I agree that it'll probably be forgotten about so your suggested documentation sounds better. If others don't have any more comments on the other patches, I'll reroll just this patch.
diff --git a/strbuf.h b/strbuf.h index 3dfeadb44c..0256114002 100644 --- a/strbuf.h +++ b/strbuf.h @@ -1,6 +1,15 @@ #ifndef STRBUF_H #define STRBUF_H +/* + * 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. + */ + struct string_list; /**
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> --- strbuf.h | 9 +++++++++ 1 file changed, 9 insertions(+)