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 <sbel...@google.com>
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 <sbel...@google.com>
---
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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
