| Message ID | 20180604162140.20688-1-peter.maydell@linaro.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | CODING_STYLE: Define our preferred form for multiline comments | expand |
On 06/04/2018 12:21 PM, Peter Maydell wrote: > The codebase has a bit of a mix of > /* multiline comments > * like this > */ > and > /* multiline comments like this > in the GNU Coding Standards style */ > > State a preference for the former. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > I admit that to some extent I'm imposing my aesthetic > preferences here; pretty sure we have a lot more style > 1 comments than style 2, though. > --- > CODING_STYLE | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/CODING_STYLE b/CODING_STYLE > index 12ba58ee293..fb1d1f1cd62 100644 > --- a/CODING_STYLE > +++ b/CODING_STYLE > @@ -124,6 +124,19 @@ 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 terminating */ on its own line: > + /* like > + * this > + */ > +Putting the initial /* on its own line is accepted, but not required. > +(Some of the existing comments in the codebase use the GNU Coding > +Standards form which does not have stars on the left; avoid this > +when writing new comments.) > + > +Rationale: Consistency, and ease of visually picking out a multiline > +comment from the surrounding code. > + > 8. trace-events style > > 8.1 0x prefix > Is there some name for the unholy abomination that is the combination of both styles?: /* Like this, but without the trailing * end-comment on a standalone line. */ I prefer the asterisks on the left because it makes it obvious when grepping that it's from a comment block; I dislike the standalone comment for taking up a bunch of room for seemingly no reason. ...Not that I expect to change your mind, or to suggest I've got enough paint for a shed this large, so any standard is better than no standard... --js
On 06/04/2018 01:21 PM, Peter Maydell wrote: > The codebase has a bit of a mix of > /* multiline comments > * like this > */ > and > /* multiline comments like this > in the GNU Coding Standards style */ > > State a preference for the former. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > I admit that to some extent I'm imposing my aesthetic > preferences here; pretty sure we have a lot more style > 1 comments than style 2, though. The later is easier to indent with unintelligent editors. Any one is fine as long as it has a guideline in coding style. > --- > CODING_STYLE | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/CODING_STYLE b/CODING_STYLE > index 12ba58ee293..fb1d1f1cd62 100644 > --- a/CODING_STYLE > +++ b/CODING_STYLE > @@ -124,6 +124,19 @@ 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 terminating */ on its own line: > + /* like > + * this > + */ > +Putting the initial /* on its own line is accepted, but not required. > +(Some of the existing comments in the codebase use the GNU Coding > +Standards form which does not have stars on the left; avoid this > +when writing new comments.) > + > +Rationale: Consistency, and ease of visually picking out a multiline > +comment from the surrounding code. Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > + > 8. trace-events style > > 8.1 0x prefix >
On Mon, 4 Jun 2018 17:21:40 +0100 Peter Maydell <peter.maydell@linaro.org> wrote: > The codebase has a bit of a mix of > /* multiline comments > * like this > */ > and > /* multiline comments like this > in the GNU Coding Standards style */ > > State a preference for the former. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > I admit that to some extent I'm imposing my aesthetic > preferences here; pretty sure we have a lot more style > 1 comments than style 2, though. > --- > CODING_STYLE | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/CODING_STYLE b/CODING_STYLE > index 12ba58ee293..fb1d1f1cd62 100644 > --- a/CODING_STYLE > +++ b/CODING_STYLE > @@ -124,6 +124,19 @@ 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 terminating */ on its own line: > + /* like > + * this > + */ > +Putting the initial /* on its own line is accepted, but not required. Could we say "at maintainer discretion", or is that always implied? The asymmetry of the proposed standard is not my favorite and a mostly blank line before and after further supports standing out from surrounding code. Note that the kernel coding style, except for certain exceptions, is: /* * This is a * multi-line * comment */ Thanks, Alex > +(Some of the existing comments in the codebase use the GNU Coding > +Standards form which does not have stars on the left; avoid this > +when writing new comments.) > + > +Rationale: Consistency, and ease of visually picking out a multiline > +comment from the surrounding code. > + > 8. trace-events style > > 8.1 0x prefix
On 05.06.2018 03:17, Alex Williamson wrote: > On Mon, 4 Jun 2018 17:21:40 +0100 > Peter Maydell <peter.maydell@linaro.org> wrote: > >> The codebase has a bit of a mix of >> /* multiline comments >> * like this >> */ >> and >> /* multiline comments like this >> in the GNU Coding Standards style */ >> >> State a preference for the former. >> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> >> --- >> I admit that to some extent I'm imposing my aesthetic >> preferences here; pretty sure we have a lot more style >> 1 comments than style 2, though. >> --- >> CODING_STYLE | 13 +++++++++++++ >> 1 file changed, 13 insertions(+) >> >> diff --git a/CODING_STYLE b/CODING_STYLE >> index 12ba58ee293..fb1d1f1cd62 100644 >> --- a/CODING_STYLE >> +++ b/CODING_STYLE >> @@ -124,6 +124,19 @@ 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 terminating */ on its own line: >> + /* like >> + * this >> + */ >> +Putting the initial /* on its own line is accepted, but not required. > > Could we say "at maintainer discretion", or is that always implied? The > asymmetry of the proposed standard is not my favorite and a mostly > blank line before and after further supports standing out from > surrounding code. I also don't like the asymmetry. I'd prefer more dense comments, though: /* like * this */ Anyway, could we either use that dense format or the kernel-style multi-lines-comment format, please? Mixing it asymmetrically is just ugly. Thomas
On Tue, 5 Jun 2018 06:33:22 +0200 Thomas Huth <thuth@redhat.com> wrote: > On 05.06.2018 03:17, Alex Williamson wrote: > > On Mon, 4 Jun 2018 17:21:40 +0100 > > Peter Maydell <peter.maydell@linaro.org> wrote: > > > >> The codebase has a bit of a mix of > >> /* multiline comments > >> * like this > >> */ > >> and > >> /* multiline comments like this > >> in the GNU Coding Standards style */ > >> > >> State a preference for the former. > >> > >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > >> --- > >> I admit that to some extent I'm imposing my aesthetic > >> preferences here; pretty sure we have a lot more style > >> 1 comments than style 2, though. > >> --- > >> CODING_STYLE | 13 +++++++++++++ > >> 1 file changed, 13 insertions(+) > >> > >> diff --git a/CODING_STYLE b/CODING_STYLE > >> index 12ba58ee293..fb1d1f1cd62 100644 > >> --- a/CODING_STYLE > >> +++ b/CODING_STYLE > >> @@ -124,6 +124,19 @@ 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 terminating */ on its own line: > >> + /* like > >> + * this > >> + */ > >> +Putting the initial /* on its own line is accepted, but not required. > > > > Could we say "at maintainer discretion", or is that always implied? The > > asymmetry of the proposed standard is not my favorite and a mostly > > blank line before and after further supports standing out from > > surrounding code. > I also don't like the asymmetry. I'd prefer more dense comments, though: > > /* like > * this */ > > Anyway, could we either use that dense format or the kernel-style > multi-lines-comment format, please? Mixing it asymmetrically is just ugly. I'd vote for the kernel style, then. I'd also like the bikeshed to be painted in a grayed blue-green.
On 5 June 2018 at 08:46, Cornelia Huck <cohuck@redhat.com> wrote: > On Tue, 5 Jun 2018 06:33:22 +0200 > Thomas Huth <thuth@redhat.com> wrote: >> On 05.06.2018 03:17, Alex Williamson wrote: >> > On Mon, 4 Jun 2018 17:21:40 +0100 >> > Peter Maydell <peter.maydell@linaro.org> wrote: >> >> +Multiline comments blocks should have a row of stars on the left >> >> +and the terminating */ on its own line: >> >> + /* like >> >> + * this >> >> + */ >> >> +Putting the initial /* on its own line is accepted, but not required. >> > >> > Could we say "at maintainer discretion", or is that always implied? The >> > asymmetry of the proposed standard is not my favorite and a mostly >> > blank line before and after further supports standing out from >> > surrounding code. >> I also don't like the asymmetry. I'd prefer more dense comments, though: >> >> /* like >> * this */ Wow, I think that looks terrible :-) >> Anyway, could we either use that dense format or the kernel-style >> multi-lines-comment format, please? Mixing it asymmetrically is just ugly. > > I'd vote for the kernel style, then. I don't particularly object to the kernel style (though it's not how I personally default to writing comments). I just didn't want to rule a huge chunk of our existing comments as out-of-standard for what I see as a relatively minor divergence in form -- we do have a lot of no-leading-separate-/* comments. I can live with mandating kernel-style if it means we can rule out GNU-form and other weirdnesses though :-) thanks -- PMM
On 05.06.2018 11:19, Peter Maydell wrote: > On 5 June 2018 at 08:46, Cornelia Huck <cohuck@redhat.com> wrote: >> On Tue, 5 Jun 2018 06:33:22 +0200 >> Thomas Huth <thuth@redhat.com> wrote: >>> On 05.06.2018 03:17, Alex Williamson wrote: >>>> On Mon, 4 Jun 2018 17:21:40 +0100 >>>> Peter Maydell <peter.maydell@linaro.org> wrote: >>>>> +Multiline comments blocks should have a row of stars on the left >>>>> +and the terminating */ on its own line: >>>>> + /* like >>>>> + * this >>>>> + */ >>>>> +Putting the initial /* on its own line is accepted, but not required. >>>> >>>> Could we say "at maintainer discretion", or is that always implied? The >>>> asymmetry of the proposed standard is not my favorite and a mostly >>>> blank line before and after further supports standing out from >>>> surrounding code. >>> I also don't like the asymmetry. I'd prefer more dense comments, though: >>> >>> /* like >>> * this */ > > Wow, I think that looks terrible :-) ... but ... but ... this color is so much better ... ? ;-) >>> Anyway, could we either use that dense format or the kernel-style >>> multi-lines-comment format, please? Mixing it asymmetrically is just ugly. >> >> I'd vote for the kernel style, then. > > I don't particularly object to the kernel style (though it's not > how I personally default to writing comments). I just didn't want > to rule a huge chunk of our existing comments as out-of-standard > for what I see as a relatively minor divergence in form -- > we do have a lot of no-leading-separate-/* comments. I can live > with mandating kernel-style if it means we can rule out GNU-form > and other weirdnesses though :-) Ok, then maybe write that kernel-style is the preferred way, but also mention that the "mixed" style is OK, too? If that helps to avoid that somebody is sending hundreds of trivial patches to the list to correct them, I'm fine with it. Thomas
On Tue, 5 Jun 2018 10:19:15 +0100 Peter Maydell <peter.maydell@linaro.org> wrote: > On 5 June 2018 at 08:46, Cornelia Huck <cohuck@redhat.com> wrote: > > On Tue, 5 Jun 2018 06:33:22 +0200 > > Thomas Huth <thuth@redhat.com> wrote: > >> Anyway, could we either use that dense format or the kernel-style > >> multi-lines-comment format, please? Mixing it asymmetrically is just ugly. > > > > I'd vote for the kernel style, then. > > I don't particularly object to the kernel style (though it's not > how I personally default to writing comments). I just didn't want > to rule a huge chunk of our existing comments as out-of-standard > for what I see as a relatively minor divergence in form -- > we do have a lot of no-leading-separate-/* comments. I can live > with mandating kernel-style if it means we can rule out GNU-form > and other weirdnesses though :-) Yes, we should be flexible... as long as it isn't GNU :) Whatever we end up with, we should take care that we don't get zillions of "cleanup" patches that don't add value... something like "no need to change preexisting comments, unless you're touching the code anyway".
Peter Maydell <peter.maydell@linaro.org> writes: > On 5 June 2018 at 08:46, Cornelia Huck <cohuck@redhat.com> wrote: >> On Tue, 5 Jun 2018 06:33:22 +0200 >> Thomas Huth <thuth@redhat.com> wrote: >>> On 05.06.2018 03:17, Alex Williamson wrote: >>> > On Mon, 4 Jun 2018 17:21:40 +0100 >>> > Peter Maydell <peter.maydell@linaro.org> wrote: >>> >> +Multiline comments blocks should have a row of stars on the left >>> >> +and the terminating */ on its own line: >>> >> + /* like >>> >> + * this >>> >> + */ Uh, winging just one end of the comment offends my eyes. >>> >> +Putting the initial /* on its own line is accepted, but not required. >>> > >>> > Could we say "at maintainer discretion", or is that always implied? The >>> > asymmetry of the proposed standard is not my favorite and a mostly >>> > blank line before and after further supports standing out from >>> > surrounding code. >>> I also don't like the asymmetry. I'd prefer more dense comments, though: >>> >>> /* like >>> * this */ > > Wow, I think that looks terrible :-) Even more terrible, you wanted to say ;) >>> Anyway, could we either use that dense format or the kernel-style >>> multi-lines-comment format, please? Mixing it asymmetrically is just ugly. >> >> I'd vote for the kernel style, then. > > I don't particularly object to the kernel style (though it's not > how I personally default to writing comments). I just didn't want > to rule a huge chunk of our existing comments as out-of-standard > for what I see as a relatively minor divergence in form -- > we do have a lot of no-leading-separate-/* comments. I can live > with mandating kernel-style if it means we can rule out GNU-form > and other weirdnesses though :-) Let's mandate kernel-style for new code. I could live with giving maintainers license to tolerate certain other styles. The fewer, the better, though.
On Mon, Jun 04, 2018 at 07:17:56PM -0600, Alex Williamson wrote: > Note that the kernel coding style, except for certain exceptions, is: > > /* > * This is a > * multi-line > * comment > */ Amen, the one true way. :-) Stefan
On 7 June 2018 at 13:40, Stefan Hajnoczi <stefanha@gmail.com> wrote: > On Mon, Jun 04, 2018 at 07:17:56PM -0600, Alex Williamson wrote: >> Note that the kernel coding style, except for certain exceptions, is: >> >> /* >> * This is a >> * multi-line >> * comment >> */ > > Amen, the one true way. :-) OK, consensus seems to be for recommending the kernel-style for multiline comments. Does anybody who knows checkpatch feel like they could write a checkpatch check for this? (Unfortunately there isn't one in the kernel's checkpatch script either, as I found out when I last wrote a kernel patch and got dinged for not putting that first blank line in...) thanks -- PMM
I'm late to the party, but anyway... Am 07.06.2018 um 14:02 hat Markus Armbruster geschrieben: > Peter Maydell <peter.maydell@linaro.org> writes: > > > On 5 June 2018 at 08:46, Cornelia Huck <cohuck@redhat.com> wrote: > >> On Tue, 5 Jun 2018 06:33:22 +0200 > >> Thomas Huth <thuth@redhat.com> wrote: > >>> On 05.06.2018 03:17, Alex Williamson wrote: > >>> > On Mon, 4 Jun 2018 17:21:40 +0100 > >>> > Peter Maydell <peter.maydell@linaro.org> wrote: > >>> >> +Multiline comments blocks should have a row of stars on the left > >>> >> +and the terminating */ on its own line: > >>> >> + /* like > >>> >> + * this > >>> >> + */ > > Uh, winging just one end of the comment offends my eyes. +1, this is the ugliest style of all. > >>> >> +Putting the initial /* on its own line is accepted, but not required. > >>> > > >>> > Could we say "at maintainer discretion", or is that always implied? The > >>> > asymmetry of the proposed standard is not my favorite and a mostly > >>> > blank line before and after further supports standing out from > >>> > surrounding code. > >>> I also don't like the asymmetry. I'd prefer more dense comments, though: > >>> > >>> /* like > >>> * this */ > > > > Wow, I think that looks terrible :-) > > Even more terrible, you wanted to say ;) I actually prefer this one for short (2 or 3 lines) not too important comments in order to save some screen space. For longer or important comments, it's kernel-style. > >>> Anyway, could we either use that dense format or the kernel-style > >>> multi-lines-comment format, please? Mixing it asymmetrically is just ugly. > >> > >> I'd vote for the kernel style, then. > > > > I don't particularly object to the kernel style (though it's not > > how I personally default to writing comments). I just didn't want > > to rule a huge chunk of our existing comments as out-of-standard > > for what I see as a relatively minor divergence in form -- > > we do have a lot of no-leading-separate-/* comments. I can live > > with mandating kernel-style if it means we can rule out GNU-form > > and other weirdnesses though :-) > > Let's mandate kernel-style for new code. I could live with giving > maintainers license to tolerate certain other styles. The fewer, the > better, though. Kernel-style + give maintainers license to tolerate more compact forms (mostly for short comments) works for me. Kevin
diff --git a/CODING_STYLE b/CODING_STYLE index 12ba58ee293..fb1d1f1cd62 100644 --- a/CODING_STYLE +++ b/CODING_STYLE @@ -124,6 +124,19 @@ 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 terminating */ on its own line: + /* like + * this + */ +Putting the initial /* on its own line is accepted, but not required. +(Some of the existing comments in the codebase use the GNU Coding +Standards form which does not have stars on the left; avoid this +when writing new comments.) + +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 /* multiline comments * like this */ and /* multiline comments like this in the GNU Coding Standards style */ State a preference for the former. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- I admit that to some extent I'm imposing my aesthetic preferences here; pretty sure we have a lot more style 1 comments than style 2, though. --- CODING_STYLE | 13 +++++++++++++ 1 file changed, 13 insertions(+) -- 2.17.1