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

Re: [PATCH] strbuf.h: format according to coding guidelines

From
Junio C Hamano <gitster@pobox.com>
Date
Sep 28, 2018, 21:38 UTC
Message-ID
<xmqq8t3lb8uu.fsf@gitster-ct.c.googlers.com>
In-Reply-To
<xmqqin2pbcwh.fsf@gitster-ct.c.googlers.com>
Junio C Hamano <gitster@pobox.com> writes:
> I actually do not mind the rule to be more like
>
>  * Use the same parameter names used in the function declaration when
>    the description in the API documentation refers the parameter.

Assuming that we adopt the above guideline, let's extending it to the original patch's review.

The following is a good example. FIRST and SECOND would have been upcased if this followed my earlier illustration to make them stand out as references to the parameters, but it is already readable without upcasing _and_ naming parameters is helping here.

 /**
  * Compare two buffers. Returns an integer less than, equal to, or greater
  * than zero if the first buffer is found, respectively, to be less than,
  * to match, or be greater than the second buffer.
  */
-extern int strbuf_cmp(const struct strbuf *, const struct strbuf *);
+int strbuf_cmp(const struct strbuf *first, const struct strbuf *second);

The next one could be improved and say something like: "Remove LEN bytes in the strbuf SB starting at offset POS", as it already had 'pos' and 'len' that are readily usable. Notice that "Remove LEN bytes starting at offset POS" is a sufficiently clear description and that is why I do not think we should require all parameters to be named.

 /**
  * Remove given amount of data from a given position of the buffer.
  */
-extern void strbuf_remove(struct strbuf *, size_t pos, size_t len);
+void strbuf_remove(struct strbuf *sb, size_t pos, size_t len);
 

The last example is a job half-done. The original had pos and len parameters and referred to them in the text, but just said "with the given data". Now we have data and data_len, "the given data" can and should be clarified by referring to them.

 /**
  * Remove the bytes between `pos..pos+len` and replace it with the given
  * data.
  */
-extern void strbuf_splice(struct strbuf *, size_t pos, size_t len,
-			  const void *, size_t);
+void strbuf_splice(struct strbuf *sb, size_t pos, size_t len,
+		   const void *data, size_t data_len);
Previous: Junio C HamanoNext: Jeff King
Message 36 of 51 in “[PATCHv3 0/8] fetch: make sure submodule oids are fetched”
  1. Stefan BellerSep 21, 2018
  2. 1/8 sha1-array: provide oid_array_filterStefan Beller, Sep 21, 2018
  3. Ævar Arnfjörð BjarmasonSep 22, 2018
  4. Stefan BellerSep 25, 2018
  5. Jeff KingSep 26, 2018
  6. Junio C HamanoSep 26, 2018
  7. Ævar Arnfjörð BjarmasonSep 26, 2018
  8. Junio C HamanoSep 26, 2018
  9. Jeff KingSep 26, 2018
  10. Ævar Arnfjörð BjarmasonSep 26, 2018
  11. Jeff KingSep 26, 2018
  12. Stefan BellerSep 26, 2018
  13. Junio C HamanoSep 26, 2018
  14. Stefan BellerSep 26, 2018
  15. Junio C HamanoSep 26, 2018
  16. Jeff KingSep 26, 2018
  17. Stefan BellerSep 26, 2018
  18. On shipping more of our technical docs as manpagesÆvar Arnfjörð Bjarmason, Sep 26, 2018
  19. Junio C HamanoSep 26, 2018
  20. Stefan BellerSep 26, 2018
  21. Ævar Arnfjörð BjarmasonSep 27, 2018
  22. Jeff KingSep 27, 2018
  23. Jonathan NiederSep 27, 2018
  24. Jeff KingSep 27, 2018
  25. Junio C HamanoSep 27, 2018
  26. Jeff KingSep 27, 2018
  27. Documentation/CodingGuidelines: How to document new APIsStefan Beller, Sep 27, 2018
  28. Junio C HamanoSep 27, 2018
  29. Ævar Arnfjörð BjarmasonSep 27, 2018
  30. Jonathan NiederSep 27, 2018
  31. Jeff KingSep 28, 2018
  32. Junio C HamanoSep 28, 2018
  33. strbuf.h: format according to coding guidelinesStefan Beller, Sep 28, 2018
  34. Junio C HamanoSep 28, 2018
  35. Junio C HamanoSep 28, 2018
  36. Junio C HamanoSep 28, 2018
  37. Jeff KingSep 29, 2018
  38. Junio C HamanoSep 28, 2018
  39. Stefan BellerSep 28, 2018
  40. Martin ÅgrenSep 28, 2018
  41. Jeff KingSep 29, 2018
  42. Stefan BellerSep 28, 2018
  43. Jeff KingSep 29, 2018
  44. Jeff KingSep 27, 2018
  45. 2/8 submodule.c: fix indentationStefan Beller, Sep 21, 2018
  46. 3/8 submodule.c: sort changed_submodule_names before searching itStefan Beller, Sep 21, 2018
  47. 4/8 submodule: move global changed_submodule_names into fetch submodule structStefan Beller, Sep 21, 2018
  48. 5/8 submodule.c: do not copy around submodule listStefan Beller, Sep 21, 2018
  49. 6/8 submodule: fetch in submodules git directory instead of in worktreeStefan Beller, Sep 21, 2018
  50. 7/8 fetch: retry fetching submodules if needed objects were not fetchedStefan Beller, Sep 21, 2018
  51. 8/8 builtin/fetch: check for submodule updates for non branch fetchesStefan Beller, Sep 21, 2018

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.