Message ID | 20180611141716.3813-1-peter.maydell@linaro.org |
---|---|
State | Superseded |
Headers | show |
Series | [v2] CODING_STYLE: Define our preferred form for multiline comments | expand |
On 06/11/2018 09:17 AM, Peter Maydell wrote: > The codebase has a bit of a mix of different multiline > comment styles. State a preference for the Linux kernel > style: > /* > * Star on the left for each line. > * Leading slash-star and trailing star-slash > * each go on a line of their own. > */ > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > This is not my personal favourite, but seemed to be the > closest we had to consensus in the mail thread for v1; > I can live with it in order to avoid getting patches which > use the styles I like even less :-) Ditto that sentiment. > --- > CODING_STYLE | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/CODING_STYLE b/CODING_STYLE > index 12ba58ee293..2d84f5f26d1 100644 > --- a/CODING_STYLE > +++ b/CODING_STYLE > @@ -124,6 +124,23 @@ We use traditional C-style /* */ comments and avoid // comments. > Rationale: The // form is valid in C99, so this is purely a matter of > consistency of style. The checkpatch script will warn you about this. > > +Multiline comments blocks should have a row of stars on the left, s/comments/comment/ > +and the initial /* and terminating */ both on their own lines: > + /* > + * like > + * this > + */ > +This is the same format required by the Linux kernel coding style. > + > +(Some of the existing comments in the codebase use the GNU Coding > +Standards form which does not have stars on the left, or other > +variations; avoid these when writing new comments, but don't worry > +about converting to the preferred form unless you're editing that > +comment anyway.) > + > +Rationale: Consistency, and ease of visually picking out a multiline > +comment from the surrounding code. > + With the grammar fix, Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On Mon, 11 Jun 2018 15:17:16 +0100 Peter Maydell <peter.maydell@linaro.org> wrote: > The codebase has a bit of a mix of different multiline > comment styles. State a preference for the Linux kernel > style: > /* > * Star on the left for each line. > * Leading slash-star and trailing star-slash > * each go on a line of their own. > */ > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > This is not my personal favourite, but seemed to be the > closest we had to consensus in the mail thread for v1; > I can live with it in order to avoid getting patches which > use the styles I like even less :-) > --- > CODING_STYLE | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Peter Maydell <peter.maydell@linaro.org> writes: > The codebase has a bit of a mix of different multiline > comment styles. State a preference for the Linux kernel > style: > /* > * Star on the left for each line. > * Leading slash-star and trailing star-slash > * each go on a line of their own. > */ > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Markus Armbruster <armbru@redhat.com>
On Mon, 11 Jun 2018 15:17:16 +0100 Peter Maydell <peter.maydell@linaro.org> wrote: > The codebase has a bit of a mix of different multiline > comment styles. State a preference for the Linux kernel > style: > /* > * Star on the left for each line. > * Leading slash-star and trailing star-slash > * each go on a line of their own. > */ > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > This is not my personal favourite, but seemed to be the > closest we had to consensus in the mail thread for v1; > I can live with it in order to avoid getting patches which > use the styles I like even less :-) > --- > CODING_STYLE | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/CODING_STYLE b/CODING_STYLE > index 12ba58ee293..2d84f5f26d1 100644 > --- a/CODING_STYLE > +++ b/CODING_STYLE > @@ -124,6 +124,23 @@ We use traditional C-style /* */ comments and avoid // comments. > Rationale: The // form is valid in C99, so this is purely a matter of > consistency of style. The checkpatch script will warn you about this. > > +Multiline comments blocks should have a row of stars on the left, > +and the initial /* and terminating */ both on their own lines: > + /* > + * like > + * this > + */ > +This is the same format required by the Linux kernel coding style. > + > +(Some of the existing comments in the codebase use the GNU Coding > +Standards form which does not have stars on the left, or other > +variations; avoid these when writing new comments, but don't worry > +about converting to the preferred form unless you're editing that > +comment anyway.) > + > +Rationale: Consistency, and ease of visually picking out a multiline > +comment from the surrounding code. > + > 8. trace-events style > > 8.1 0x prefix Much preferred, thanks (tabs next? ;) Reviewed-by: Alex Williamson <alex.williamson@redhat.com>
On 11.06.2018 16:17, Peter Maydell wrote: > The codebase has a bit of a mix of different multiline > comment styles. State a preference for the Linux kernel > style: > /* > * Star on the left for each line. > * Leading slash-star and trailing star-slash > * each go on a line of their own. > */ > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > This is not my personal favourite, but seemed to be the > closest we had to consensus in the mail thread for v1; > I can live with it in order to avoid getting patches which > use the styles I like even less :-) > --- > CODING_STYLE | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/CODING_STYLE b/CODING_STYLE > index 12ba58ee293..2d84f5f26d1 100644 > --- a/CODING_STYLE > +++ b/CODING_STYLE > @@ -124,6 +124,23 @@ We use traditional C-style /* */ comments and avoid // comments. > Rationale: The // form is valid in C99, so this is purely a matter of > consistency of style. The checkpatch script will warn you about this. > > +Multiline comments blocks should have a row of stars on the left, > +and the initial /* and terminating */ both on their own lines: > + /* > + * like > + * this > + */ > +This is the same format required by the Linux kernel coding style. > + > +(Some of the existing comments in the codebase use the GNU Coding > +Standards form which does not have stars on the left, or other > +variations; avoid these when writing new comments, but don't worry > +about converting to the preferred form unless you're editing that > +comment anyway.) > + > +Rationale: Consistency, and ease of visually picking out a multiline > +comment from the surrounding code. s/multiline/multi-line/g maybe? That seems to be the more common spelling...? Anyway: Reviewed-by: Thomas Huth <thuth@redhat.com>
On 06/11/2018 10:17 AM, Peter Maydell wrote: > The codebase has a bit of a mix of different multiline > comment styles. State a preference for the Linux kernel > style: > /* > * Star on the left for each line. > * Leading slash-star and trailing star-slash > * each go on a line of their own. > */ > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > This is not my personal favourite, but seemed to be the > closest we had to consensus in the mail thread for v1; > I can live with it in order to avoid getting patches which > use the styles I like even less :-) "Same." Well, at least it's not asymmetric. Reviewed-by: John Snow <jsnow@redhat.com> > --- > CODING_STYLE | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/CODING_STYLE b/CODING_STYLE > index 12ba58ee293..2d84f5f26d1 100644 > --- a/CODING_STYLE > +++ b/CODING_STYLE > @@ -124,6 +124,23 @@ We use traditional C-style /* */ comments and avoid // comments. > Rationale: The // form is valid in C99, so this is purely a matter of > consistency of style. The checkpatch script will warn you about this. > > +Multiline comments blocks should have a row of stars on the left, > +and the initial /* and terminating */ both on their own lines: > + /* > + * like > + * this > + */ > +This is the same format required by the Linux kernel coding style. > + > +(Some of the existing comments in the codebase use the GNU Coding > +Standards form which does not have stars on the left, or other > +variations; avoid these when writing new comments, but don't worry > +about converting to the preferred form unless you're editing that > +comment anyway.) > + > +Rationale: Consistency, and ease of visually picking out a multiline > +comment from the surrounding code. > + > 8. trace-events style > > 8.1 0x prefix >
On 11 June 2018 at 17:15, Alex Williamson <alex.williamson@redhat.com> wrote:
> Much preferred, thanks (tabs next? ;)
CODING_STYLE already mandates spaces-only, no-tabs...
thanks
-- PMM
On Mon, 11 Jun 2018 17:40:35 +0100 Peter Maydell <peter.maydell@linaro.org> wrote: > On 11 June 2018 at 17:15, Alex Williamson <alex.williamson@redhat.com> wrote: > > Much preferred, thanks (tabs next? ;) > > CODING_STYLE already mandates spaces-only, no-tabs... That was a joke, note the smiley. I know, I just don't like it. Thanks, Alex
On 06/11/2018 04:17 AM, Peter Maydell wrote: > The codebase has a bit of a mix of different multiline > comment styles. State a preference for the Linux kernel > style: > /* > * Star on the left for each line. > * Leading slash-star and trailing star-slash > * each go on a line of their own. > */ > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > This is not my personal favourite, but seemed to be the > closest we had to consensus in the mail thread for v1; > I can live with it in order to avoid getting patches which > use the styles I like even less :-) Honestly, I don't like this except for "important" comments. A "small" comment, e.g. one that doesn't quite fit on a single line, now takes 4 lines instead of 2. Which is really annoying and IMO tends to break flow. If you don't like /* gnu style */ or /* whatever * this is */ could you live with // c99/c++ // comments (I know we currently deny those in checkpatch, but that's easy enough to remove if we're changing policy. I have no idea why we don't like them in the first place. Or other c99 features like mid-block or control declarations, for that matter.) r~
On 06/12/2018 02:30 PM, Richard Henderson wrote: > On 06/11/2018 04:17 AM, Peter Maydell wrote: >> The codebase has a bit of a mix of different multiline >> comment styles. State a preference for the Linux kernel >> style: >> /* >> * Star on the left for each line. >> * Leading slash-star and trailing star-slash >> * each go on a line of their own. >> */ >> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> >> --- >> This is not my personal favourite, but seemed to be the >> closest we had to consensus in the mail thread for v1; >> I can live with it in order to avoid getting patches which >> use the styles I like even less :-) > > Honestly, I don't like this except for "important" comments. > > A "small" comment, e.g. one that doesn't quite fit on a single line, now takes > 4 lines instead of 2. Which is really annoying and IMO tends to break flow. > > If you don't like > > /* gnu > style */ > > or > > /* whatever > * this is */ > > could you live with > > // c99/c++ > // comments > > (I know we currently deny those in checkpatch, but that's easy enough to remove > if we're changing policy. I have no idea why we don't like them in the first > place. Or other c99 features like mid-block or control declarations, for that > matter.) +1 for control declarations :) Code is easier to read and keep focus on. 3.0 release is a good opportunity to establish such disruptive changes.
On 12.06.2018 19:30, Richard Henderson wrote: > On 06/11/2018 04:17 AM, Peter Maydell wrote: >> The codebase has a bit of a mix of different multiline >> comment styles. State a preference for the Linux kernel >> style: >> /* >> * Star on the left for each line. >> * Leading slash-star and trailing star-slash >> * each go on a line of their own. >> */ >> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> >> --- >> This is not my personal favourite, but seemed to be the >> closest we had to consensus in the mail thread for v1; >> I can live with it in order to avoid getting patches which >> use the styles I like even less :-) > > Honestly, I don't like this except for "important" comments. > > A "small" comment, e.g. one that doesn't quite fit on a single line, now takes > 4 lines instead of 2. Which is really annoying and IMO tends to break flow. > > If you don't like > > /* gnu > style */ > > or > > /* whatever > * this is */ > > could you live with > > // c99/c++ > // comments FWIW: +1 for one or two of those compact styles for two- or three-line comments. Thomas
On Tue, 12 Jun 2018 20:12:02 +0200 Thomas Huth <thuth@redhat.com> wrote: > On 12.06.2018 19:30, Richard Henderson wrote: > > On 06/11/2018 04:17 AM, Peter Maydell wrote: > >> The codebase has a bit of a mix of different multiline > >> comment styles. State a preference for the Linux kernel > >> style: > >> /* > >> * Star on the left for each line. > >> * Leading slash-star and trailing star-slash > >> * each go on a line of their own. > >> */ > >> > >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > >> --- > >> This is not my personal favourite, but seemed to be the > >> closest we had to consensus in the mail thread for v1; > >> I can live with it in order to avoid getting patches which > >> use the styles I like even less :-) > > > > Honestly, I don't like this except for "important" comments. > > > > A "small" comment, e.g. one that doesn't quite fit on a single line, now takes > > 4 lines instead of 2. Which is really annoying and IMO tends to break flow. > > > > If you don't like > > > > /* gnu > > style */ > > > > or > > > > /* whatever > > * this is */ > > > > could you live with > > > > // c99/c++ > > // comments > > FWIW: > > +1 for one or two of those compact styles for two- or three-line comments. or four-line or single paragraphs or comments that rhyme with "orange"... If the comment is too large for a single line, then take some time to say it more concisely, or maybe it does deserve enough thought to frame a nice paragraph for it. We're well into personal style here, so either this is important enough to make some people unhappy or we should leave it to maintainer preference and consistency within a file/area. Thanks, Alex
On Tue, 12 Jun 2018 12:47:29 -0600 Alex Williamson <alex.williamson@redhat.com> wrote: > On Tue, 12 Jun 2018 20:12:02 +0200 > Thomas Huth <thuth@redhat.com> wrote: > > > On 12.06.2018 19:30, Richard Henderson wrote: > > > On 06/11/2018 04:17 AM, Peter Maydell wrote: > > >> The codebase has a bit of a mix of different multiline > > >> comment styles. State a preference for the Linux kernel > > >> style: > > >> /* > > >> * Star on the left for each line. > > >> * Leading slash-star and trailing star-slash > > >> * each go on a line of their own. > > >> */ > > >> > > >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > > >> --- > > >> This is not my personal favourite, but seemed to be the > > >> closest we had to consensus in the mail thread for v1; > > >> I can live with it in order to avoid getting patches which > > >> use the styles I like even less :-) > > > > > > Honestly, I don't like this except for "important" comments. > > > > > > A "small" comment, e.g. one that doesn't quite fit on a single line, now takes > > > 4 lines instead of 2. Which is really annoying and IMO tends to break flow. > > > > > > If you don't like > > > > > > /* gnu > > > style */ > > > > > > or > > > > > > /* whatever > > > * this is */ > > > > > > could you live with > > > > > > // c99/c++ > > > // comments > > > > FWIW: > > > > +1 for one or two of those compact styles for two- or three-line comments. > > or four-line or single paragraphs or comments that rhyme with > "orange"... If the comment is too large for a single line, then take > some time to say it more concisely, or maybe it does deserve enough > thought to frame a nice paragraph for it. We're well into personal > style here, so either this is important enough to make some people > unhappy or we should leave it to maintainer preference and consistency > within a file/area. Thanks, +1 to maintainer judgment and consistency. (And FWIW, c++ style comments are what I use when I comment something out for debugging, so I don't like them in regular code :)
On 12 June 2018 at 18:30, Richard Henderson <richard.henderson@linaro.org> wrote: > On 06/11/2018 04:17 AM, Peter Maydell wrote: >> The codebase has a bit of a mix of different multiline >> comment styles. State a preference for the Linux kernel >> style: >> /* >> * Star on the left for each line. >> * Leading slash-star and trailing star-slash >> * each go on a line of their own. >> */ >> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> >> --- >> This is not my personal favourite, but seemed to be the >> closest we had to consensus in the mail thread for v1; >> I can live with it in order to avoid getting patches which >> use the styles I like even less :-) > > Honestly, I don't like this except for "important" comments. > > A "small" comment, e.g. one that doesn't quite fit on a single line, now takes > 4 lines instead of 2. Which is really annoying and IMO tends to break flow. That's why I prefer /* two * lines */ style over kernel-style. But apparently nobody else does... thanks -- PMM
On Mon, Jun 11, 2018 at 03:17:16PM +0100, Peter Maydell wrote: > The codebase has a bit of a mix of different multiline > comment styles. State a preference for the Linux kernel > style: > /* > * Star on the left for each line. > * Leading slash-star and trailing star-slash > * each go on a line of their own. > */ > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > This is not my personal favourite, but seemed to be the > closest we had to consensus in the mail thread for v1; > I can live with it in order to avoid getting patches which > use the styles I like even less :-) > --- > CODING_STYLE | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
On 06/13/2018 08:30 AM, Peter Maydell wrote: > On 12 June 2018 at 18:30, Richard Henderson > <richard.henderson@linaro.org> wrote: >> On 06/11/2018 04:17 AM, Peter Maydell wrote: >>> The codebase has a bit of a mix of different multiline >>> comment styles. State a preference for the Linux kernel >>> style: >>> /* >>> * Star on the left for each line. >>> * Leading slash-star and trailing star-slash >>> * each go on a line of their own. >>> */ >>> >>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> >>> --- >>> This is not my personal favourite, but seemed to be the >>> closest we had to consensus in the mail thread for v1; >>> I can live with it in order to avoid getting patches which >>> use the styles I like even less :-) >> >> Honestly, I don't like this except for "important" comments. >> >> A "small" comment, e.g. one that doesn't quite fit on a single line, now takes >> 4 lines instead of 2. Which is really annoying and IMO tends to break flow. > > That's why I prefer > /* two > * lines > */ > style over kernel-style. But apparently nobody else does... The same reasoning could be used to justify /* two * lines */ as it's ... actually just two lines. I think people don't seem to like this much either (why? does it look 'naked' on the end?) and seemingly the asymmetry is a bit of a killer for the isolated close-comment style: /* asymmetric * three lines */ so we seemed to have arrived on the vertically symmetric: /* * four * lines */ style. I think this is a pretty reasonable compromise with the tastes of the various maintainers that have chimed in so far -- it seems to be nobody's favorite but everyone's second choice :) If there is concern this will balloon "almost" single line comments into four lines, I think the overall strategy of being either more concise (or being 70 characters more detailed!) is probably reasonable enough. ...Overall I think it probably doesn't matter too much as long as comments in each individual file are consistent. It would only begin to matter terribly much if we actually decided we wanted to do a doxygen-style doc generation for our internal APIs for compatibility with, say, fancier IDEs than vim/emacs. As it stands, we're pretty inconsistent about which exact style we apply when we "document" internal functions -- sometimes we document the header, sometimes the implementation, sometimes both (but differently!) and always with different styles all over the place. That's the real problem, IMO. For non-javadoc style comments that aren't intended to document a function in some methodical way, I think as long as it looks nice in the file as deemed by the maintainer and it's consistent, it's probably fine. > > thanks > -- PMM > Now I can say after working at Red Hat for four years that I've finally had a real life "tabs vs spaces" argument, too. I guess I had a good run without :) --js
On 13 June 2018 at 17:55, John Snow <jsnow@redhat.com> wrote: > The same reasoning could be used to justify > > /* two > * lines */ > > as it's ... actually just two lines. I think people don't seem to like > this much either (why? does it look 'naked' on the end?) I dislike the way it breaks up the line of stars. For me it is the /* * */ shape that defines a multiline comment, and where exactly the text is on the RHS of it is not important to my sense of visual neatness :-) > It would only begin to matter terribly much if we actually decided we > wanted to do a doxygen-style doc generation for our internal APIs for > compatibility with, say, fancier IDEs than vim/emacs. We ought to do that at some point -- I had some prototype patches for it. Doc-comment comments always start /** on a line of its own, though. > As it stands, we're pretty inconsistent about which exact style we apply > when we "document" internal functions -- sometimes we document the > header, sometimes the implementation, sometimes both (but differently!) > and always with different styles all over the place. That's the real > problem, IMO. IMHO -- global functions should always be documented in the header with the prototype, and any new global function should get a doc comment (I require this for code I review...) I should be able to read about the API your code exposes to the rest of QEMU purely by looking at your headers. thanks -- PMM
On 06/14/2018 06:46 AM, Peter Maydell wrote: > On 13 June 2018 at 17:55, John Snow <jsnow@redhat.com> wrote: >> The same reasoning could be used to justify >> >> /* two >> * lines */ >> >> as it's ... actually just two lines. I think people don't seem to like >> this much either (why? does it look 'naked' on the end?) > > I dislike the way it breaks up the line of stars. For me it is the > /* > * > */ > shape that defines a multiline comment, and where exactly the text is > on the RHS of it is not important to my sense of visual neatness :-) > Yours does look an awful lot more symmetrical once you remove the text, yeah. *cough* I hate the way it looks too, but C99 comments have a few things going for them: // A multi-line comment block like this has no extra lines and every // line in the comment is prefaced individually which aids grep // readability, while maintained good vertical symmetry. I think we hate C99 comments, though? Certainly we don't use them at all right now, so it's not a good fit. >> It would only begin to matter terribly much if we actually decided we >> wanted to do a doxygen-style doc generation for our internal APIs for >> compatibility with, say, fancier IDEs than vim/emacs. > > We ought to do that at some point -- I had some prototype patches > for it. Doc-comment comments always start /** on a line of its own, > though. > I'd love this! I love vim/emacs, but my usage of it is not wizard-tier and in the past when working on large C++ projects I have benefited from the magical refactoring click-buttons, tool-tips and etc. These operations are infrequent enough that I believe it's reasonable to not know how to do them in traditional CLI editors. If we want to lure in new contributors, maybe this could sweeten the pot a bit? Rigorous, mechanically verifiable function documentation is quite nice to have in these cases. It'd be nice in general, really. It would go a long way to help us attract less "hardcore" developers implementing devices and features for QEMU without such a steep onboarding curve. Do you have a proposed standard / do we have some consensus on which generator tool or doc format we'd most like to see in QEMU? I could put in some elbow grease to shine up the block layer if so... >> As it stands, we're pretty inconsistent about which exact style we apply >> when we "document" internal functions -- sometimes we document the >> header, sometimes the implementation, sometimes both (but differently!) >> and always with different styles all over the place. That's the real >> problem, IMO. > > IMHO -- global functions should always be documented in the header > with the prototype, and any new global function should get a > doc comment (I require this for code I review...) I should be able > to read about the API your code exposes to the rest of QEMU purely > by looking at your headers. > This makes sense, though the way C code is laid out makes it unfortunate you don't get to see the same comment right beside the implementation if that's what you're working on -- but I suppose this is why we have tabs, multi-monitors and IDEs with tooltips. > thanks > -- PMM >
On 06/14/2018 05:11 PM, John Snow wrote: > On 06/14/2018 06:46 AM, Peter Maydell wrote: >> On 13 June 2018 at 17:55, John Snow <jsnow@redhat.com> wrote: >>> The same reasoning could be used to justify >>> >>> /* two >>> * lines */ >>> >>> as it's ... actually just two lines. I think people don't seem to like >>> this much either (why? does it look 'naked' on the end?) >> >> I dislike the way it breaks up the line of stars. For me it is the >> /* >> * >> */ >> shape that defines a multiline comment, and where exactly the text is >> on the RHS of it is not important to my sense of visual neatness :-) >> > > Yours does look an awful lot more symmetrical once you remove the text, > yeah. > > *cough* I hate the way it looks too, but C99 comments have a few things > going for them: > > // A multi-line comment block like this has no extra lines and every > // line in the comment is prefaced individually which aids grep > // readability, while maintained good vertical symmetry. > > I think we hate C99 comments, though? Certainly we don't use them at all > right now, so it's not a good fit. > >>> It would only begin to matter terribly much if we actually decided we >>> wanted to do a doxygen-style doc generation for our internal APIs for >>> compatibility with, say, fancier IDEs than vim/emacs. >> >> We ought to do that at some point -- I had some prototype patches >> for it. Doc-comment comments always start /** on a line of its own, >> though. >> > > I'd love this! I love vim/emacs, but my usage of it is not wizard-tier > and in the past when working on large C++ projects I have benefited from > the magical refactoring click-buttons, tool-tips and etc. These > operations are infrequent enough that I believe it's reasonable to not > know how to do them in traditional CLI editors. If we want to lure in > new contributors, maybe this could sweeten the pot a bit? > > Rigorous, mechanically verifiable function documentation is quite nice > to have in these cases. It'd be nice in general, really. It would go a > long way to help us attract less "hardcore" developers implementing > devices and features for QEMU without such a steep onboarding curve. > > Do you have a proposed standard / do we have some consensus on which > generator tool or doc format we'd most like to see in QEMU? I could put > in some elbow grease to shine up the block layer if so... > >>> As it stands, we're pretty inconsistent about which exact style we apply >>> when we "document" internal functions -- sometimes we document the >>> header, sometimes the implementation, sometimes both (but differently!) >>> and always with different styles all over the place. That's the real >>> problem, IMO. >> >> IMHO -- global functions should always be documented in the header >> with the prototype, and any new global function should get a >> doc comment (I require this for code I review...) I should be able >> to read about the API your code exposes to the rest of QEMU purely >> by looking at your headers. >> > > This makes sense, though the way C code is laid out makes it unfortunate > you don't get to see the same comment right beside the implementation if > that's what you're working on -- but I suppose this is why we have tabs, > multi-monitors and IDEs with tooltips. Thanks to tabs we don't need multi-monitors of 1600+ resolution to fit 80 chars per line.
On 14.06.2018 22:11, John Snow wrote: > > On 06/14/2018 06:46 AM, Peter Maydell wrote: [...] > > *cough* I hate the way it looks too, but C99 comments have a few things > going for them: > > // A multi-line comment block like this has no extra lines and every > // line in the comment is prefaced individually which aids grep > // readability, while maintained good vertical symmetry. > > I think we hate C99 comments, though? Certainly we don't use them at all > right now, so it's not a good fit. But why do we hate them? I remember there were some reasons for not using them 10 or 15 years ago (some old C-compilers still did not support them yet), but today? ... it's likely just a legacy feeling. Maybe it's time to get over that feeling now? >>> It would only begin to matter terribly much if we actually decided we >>> wanted to do a doxygen-style doc generation for our internal APIs for >>> compatibility with, say, fancier IDEs than vim/emacs. [...] > Do you have a proposed standard / do we have some consensus on which > generator tool or doc format we'd most like to see in QEMU? I could put > in some elbow grease to shine up the block layer if so... I remember that some years ago, somebody (I forgot who it was, sorry) once told me that we should use the same format in QEMU as in glib, i.e. GTK-Doc. But citing the GTK-Doc homepage: "For a more polished general-purpose documentation tool you may want to look at Doxygen" - so maybe that's the better choice instead. Thomas
On 15 June 2018 at 05:43, Thomas Huth <thuth@redhat.com> wrote: > On 14.06.2018 22:11, John Snow wrote: >> Do you have a proposed standard / do we have some consensus on which >> generator tool or doc format we'd most like to see in QEMU? I could put >> in some elbow grease to shine up the block layer if so... > > I remember that some years ago, somebody (I forgot who it was, sorry) > once told me that we should use the same format in QEMU as in glib, i.e. > GTK-Doc. But citing the GTK-Doc homepage: "For a more polished > general-purpose documentation tool you may want to look at Doxygen" - so > maybe that's the better choice instead. The last round of email threads on this basically concluded that we should use sphinx, which is what the kernel uses. So kernel-doc comment format, and rst for standalone docs files. Some tidying up of our current doc comments would be required (which are a bit of a mishmash of styles plus syntax errors because we've never done any kind of processing of them to keep us from making mistakes). thanks -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On 13 June 2018 at 17:55, John Snow <jsnow@redhat.com> wrote: [...] >> It would only begin to matter terribly much if we actually decided we >> wanted to do a doxygen-style doc generation for our internal APIs for >> compatibility with, say, fancier IDEs than vim/emacs. > > We ought to do that at some point -- I had some prototype patches > for it. Doc-comment comments always start /** on a line of its own, > though. > >> As it stands, we're pretty inconsistent about which exact style we apply >> when we "document" internal functions -- sometimes we document the >> header, sometimes the implementation, sometimes both (but differently!) >> and always with different styles all over the place. That's the real >> problem, IMO. > > IMHO -- global functions should always be documented in the header > with the prototype, and any new global function should get a > doc comment (I require this for code I review...) I should be able > to read about the API your code exposes to the rest of QEMU purely > by looking at your headers. Putting the function contract far from the actual function is a proven way to end up with a contract that is far from what the function actually does. With a documentation generator such as Sphinx, the usual argument for putting the contracts in headers "I want API documentation in one place, without clutter" carries a lot less weight: the generated documentation is exactly that.
diff --git a/CODING_STYLE b/CODING_STYLE index 12ba58ee293..2d84f5f26d1 100644 --- a/CODING_STYLE +++ b/CODING_STYLE @@ -124,6 +124,23 @@ We use traditional C-style /* */ comments and avoid // comments. Rationale: The // form is valid in C99, so this is purely a matter of consistency of style. The checkpatch script will warn you about this. +Multiline comments blocks should have a row of stars on the left, +and the initial /* and terminating */ both on their own lines: + /* + * like + * this + */ +This is the same format required by the Linux kernel coding style. + +(Some of the existing comments in the codebase use the GNU Coding +Standards form which does not have stars on the left, or other +variations; avoid these when writing new comments, but don't worry +about converting to the preferred form unless you're editing that +comment anyway.) + +Rationale: Consistency, and ease of visually picking out a multiline +comment from the surrounding code. + 8. trace-events style 8.1 0x prefix
The codebase has a bit of a mix of different multiline comment styles. State a preference for the Linux kernel style: /* * Star on the left for each line. * Leading slash-star and trailing star-slash * each go on a line of their own. */ Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- This is not my personal favourite, but seemed to be the closest we had to consensus in the mail thread for v1; I can live with it in order to avoid getting patches which use the styles I like even less :-) --- CODING_STYLE | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) -- 2.17.1