On Fri, Dec 12, 2014 at 04:27:26PM -0500, Jeff King wrote: > On Fri, Dec 12, 2014 at 11:35:52AM -0800, Jonathan Nieder wrote: > > > If I were doing it, I would first de-asciidoc within technical/ and > > then move into the header in a separate patch. Or first move with > > asciidoc intact and then de-asciidoc in a separate patch. Combining > > the two into a single patch is also fine. Please don't change wording > > at the same time. > > Here's what I came up with. The first patch probably does more than > you'd like (and more than I would have done if I were starting from > scratch today). But I didn't want to get into undoing the stripping of > each function-name list item that I showed earlier, as it would be a lot > of tedious manual work. If we decide we want to keep those, I'm happy to > do the work to restore them, but it didn't seem like a good tradeoff > just to create an intermediate state to make the patch prettier. > > I did split out some of the other formatting decisions, though, so they > can be evaluated individually. > > [1/4]: strbuf: migrate api-strbuf.txt documentation to strbuf.h (optional nit): The subject of this patch is slightly different than the others. How about: strbuf.h: integrate api-strbuf.txt documentation > [2/4]: strbuf.h: drop asciidoc list formatting from API docs > [3/4]: strbuf.h: format asciidoc code blocks as 4-space indent > [4/4]: strbuf.h: reorganize api function grouping headers > > -Peff I have reviewed the series and I personally like it as I am more often in the learners/discovery stage than the fast lookup stage as Junio mentioned. Another inconsistency I found specifically related to the strbuf.h documentation is the apparent changeing style. At the beginning we have extern void strbuf_attach(struct strbuf *, void *, size_t, size_t); Even with the documentation attached, which size_t is the length and what the amount of allocated memory? Later we have more explaining names, such as extern struct strbuf **strbuf_split_buf(const char *, size_t, int terminator, int max); Please append the following patch to your series. I noticed this as the text editor I use uses different colors for /** comments starting with double stars used with i.e. doxygen */ /* random comments in the file */ Thanks, Stefan -----------8<------------------ >From 5d0f98aba65e8b1415ebfcd8e06b67203e9305a7 Mon Sep 17 00:00:00 2001 From: Stefan Beller <sbeller@xxxxxxxxxx> Date: Fri, 12 Dec 2014 14:27:15 -0800 Subject: [PATCH] strbuf.h: Unify documentation comments beginnings We want to preserve the meaning of "/**" to start a documentary in depth comment, so all of the documenting comments should start with a "/**". Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx> --- strbuf.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/strbuf.h b/strbuf.h index 8649a0a..95d49e1 100644 --- a/strbuf.h +++ b/strbuf.h @@ -438,7 +438,7 @@ static inline int strbuf_strip_suffix(struct strbuf *sb, const char *suffix) return 0; } -/* +/** * Split str (of length slen) at the specified terminator character. * Return a null-terminated array of pointers to strbuf objects * holding the substrings. The substrings include the terminator, @@ -454,7 +454,7 @@ static inline int strbuf_strip_suffix(struct strbuf *sb, const char *suffix) extern struct strbuf **strbuf_split_buf(const char *, size_t, int terminator, int max); -/* +/** * Split a NUL-terminated string at the specified terminator * character. See strbuf_split_buf() for more information. */ @@ -464,7 +464,7 @@ static inline struct strbuf **strbuf_split_str(const char *str, return strbuf_split_buf(str, strlen(str), terminator, max); } -/* +/** * Split a strbuf at the specified terminator character. See * strbuf_split_buf() for more information. */ @@ -474,7 +474,7 @@ static inline struct strbuf **strbuf_split_max(const struct strbuf *sb, return strbuf_split_buf(sb->buf, sb->len, terminator, max); } -/* +/** * Split a strbuf at the specified terminator character. See * strbuf_split_buf() for more information. */ @@ -484,7 +484,7 @@ static inline struct strbuf **strbuf_split(const struct strbuf *sb, return strbuf_split_max(sb, terminator, 0); } -/* +/** * Free a NULL-terminated list of strbufs (for example, the return * values of the strbuf_split*() functions). */ @@ -492,7 +492,7 @@ extern void strbuf_list_free(struct strbuf **); extern void strbuf_add_lines(struct strbuf *sb, const char *prefix, const char *buf, size_t size); -/* +/** * Append s to sb, with the characters '<', '>', '&' and '"' converted * into XML entities. */ -- 2.2.0.38.g71d6cb7 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html