git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Junio C Hamano <gitster@pobox.com>
Date
May 10, 2023, 22:51 UTC
Message-ID
<xmqqr0rn241m.fsf@gitster.g>
In-Reply-To
<CAPig+cTQg7XzORPHeD79aHEi1ggOjTPw9X02VPgxcV9uoBOBxg@mail.gmail.com>
Eric Sunshine <sunshine@sunshineco.com> writes:
Show 41 quoted lines
> On Mon, May 8, 2023 at 1:05 PM Calvin Wan <calvinwan@google.com> 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 <calvinwan@google.com>
>> ---
>> 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.
Previous: Eric SunshineNext: Calvin Wan
Message 43 of 85 in “strbuf cleanups”
  1. 0/6 strbuf cleanupsCalvin Wan, May 2, 2023
  2. 1/6 abspath: move related functions to abspathCalvin Wan, May 2, 2023
  3. Junio C HamanoMay 2, 2023
  4. 2/6 credential-store: move related functions to credential-store fileCalvin Wan, May 2, 2023
  5. Junio C HamanoMay 2, 2023
  6. Jeff KingMay 3, 2023
  7. Jeff KingMay 3, 2023
  8. Calvin WanMay 3, 2023
  9. 3/6 object-name: move related functions to object-nameCalvin Wan, May 2, 2023
  10. 4/6 path: move related function to pathCalvin Wan, May 2, 2023
  11. 5/6 strbuf: clarify dependencyCalvin Wan, May 2, 2023
  12. Elijah NewrenMay 3, 2023
  13. 6/6 strbuf: remove environment variablesCalvin Wan, May 2, 2023
  14. Elijah NewrenMay 3, 2023
  15. Junio C HamanoMay 2, 2023
  16. Junio C HamanoMay 2, 2023
  17. Felipe ContrerasMay 2, 2023
  18. Calvin WanMay 2, 2023
  19. Elijah NewrenMay 3, 2023
  20. Calvin WanMay 3, 2023
  21. Elijah NewrenMay 7, 2023
  22. Jeff KingMay 7, 2023
  23. 0/7 strbuf cleanupsCalvin Wan, May 3, 2023
  24. 1/7 strbuf: clarify API boundaryCalvin Wan, May 3, 2023
  25. 2/7 abspath: move related functions to abspathCalvin Wan, May 3, 2023
  26. 3/7 credential-store: move related functions to credential-store fileCalvin Wan, May 3, 2023
  27. 5/7 path: move related function to pathCalvin Wan, May 3, 2023
  28. 4/7 object-name: move related functions to object-nameCalvin Wan, May 3, 2023
  29. 6/7 strbuf: clarify dependencyCalvin Wan, May 3, 2023
  30. Junio C HamanoMay 3, 2023
  31. 7/7 strbuf: remove environment variablesCalvin Wan, May 3, 2023
  32. Junio C HamanoMay 3, 2023
  33. Calvin WanMay 3, 2023
  34. Junio C HamanoMay 3, 2023
  35. 7/7 strbuf: remove environment variableCalvin Wan, May 3, 2023
  36. Junio C HamanoMay 5, 2023
  37. Calvin WanMay 8, 2023
  38. Elijah NewrenMay 7, 2023
  39. Felipe ContrerasMay 7, 2023
  40. 0/7 strbuf cleanupsCalvin Wan, May 8, 2023
  41. 1/7 strbuf: clarify API boundaryCalvin Wan, May 8, 2023
  42. Eric SunshineMay 8, 2023
  43. Junio C HamanoMay 10, 2023
  44. 3/7 credential-store: move related functions to credential-store fileCalvin Wan, May 8, 2023
  45. 5/7 path: move related function to pathCalvin Wan, May 8, 2023
  46. 2/7 abspath: move related functions to abspathCalvin Wan, May 8, 2023
  47. 4/7 object-name: move related functions to object-nameCalvin Wan, May 8, 2023
  48. 6/7 strbuf: clarify dependencyCalvin Wan, May 8, 2023
  49. 7/7 strbuf: remove global variableCalvin Wan, May 8, 2023
  50. Phillip WoodMay 10, 2023
  51. Elijah NewrenMay 9, 2023
  52. Felipe ContrerasMay 9, 2023
  53. 0/7 strbuf cleanupsCalvin Wan, May 11, 2023
  54. 1/7 strbuf: clarify API boundaryCalvin Wan, May 11, 2023
  55. Eric SunshineMay 11, 2023
  56. Calvin WanMay 11, 2023
  57. 2/7 abspath: move related functions to abspathCalvin Wan, May 11, 2023
  58. 3/7 credential-store: move related functions to credential-store fileCalvin Wan, May 11, 2023
  59. 6/7 strbuf: clarify dependencyCalvin Wan, May 11, 2023
  60. 5/7 path: move related function to pathCalvin Wan, May 11, 2023
  61. 4/7 object-name: move related functions to object-nameCalvin Wan, May 11, 2023
  62. 7/7 strbuf: remove global variableCalvin Wan, May 11, 2023
  63. Eric SunshineMay 11, 2023
  64. Junio C HamanoMay 11, 2023
  65. Phillip WoodMay 12, 2023
  66. Phillip WoodMay 12, 2023
  67. Junio C HamanoMay 12, 2023
  68. 0/7 strbuf cleanupsCalvin Wan, May 12, 2023
  69. 1/7 strbuf: clarify API boundaryCalvin Wan, May 12, 2023
  70. 2/7 abspath: move related functions to abspathCalvin Wan, May 12, 2023
  71. 4/7 object-name: move related functions to object-nameCalvin Wan, May 12, 2023
  72. 3/7 credential-store: move related functions to credential-store fileCalvin Wan, May 12, 2023
  73. 5/7 path: move related function to pathCalvin Wan, May 12, 2023
  74. 6/7 strbuf: clarify dependencyCalvin Wan, May 12, 2023
  75. 7/7 strbuf: remove global variableCalvin Wan, May 12, 2023
  76. Junio C HamanoMay 12, 2023
  77. Eric SunshineMay 13, 2023
  78. 0/7 strbuf cleanupsCalvin Wan, Jun 6, 2023
  79. 1/7 strbuf: clarify API boundaryCalvin Wan, Jun 6, 2023
  80. 2/7 strbuf: clarify dependencyCalvin Wan, Jun 6, 2023
  81. 6/7 path: move related function to pathCalvin Wan, Jun 6, 2023
  82. 3/7 abspath: move related functions to abspathCalvin Wan, Jun 6, 2023
  83. 5/7 object-name: move related functions to object-nameCalvin Wan, Jun 6, 2023
  84. 4/7 credential-store: move related functions to credential-store fileCalvin Wan, Jun 6, 2023
  85. 7/7 strbuf: remove global variableCalvin Wan, Jun 6, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.