[Qemu-devel] [PATCH] CODING_STYLE: Define our preferred form for multiline comments

[Qemu-devel] [PATCH] CODING_STYLE: Define our preferred form for multiline comments

[Peter Maydell]

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
-- 
2.17.1

Re: [Qemu-devel] [PATCH] CODING_STYLE: Define our preferred form for multiline comments

[John Snow]

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

Re: [Qemu-devel] [PATCH] CODING_STYLE: Define our preferred form for multiline comments

[Philippe Mathieu-Daudé]

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
> 

Re: [Qemu-devel] [PATCH] CODING_STYLE: Define our preferred form for multiline comments

[Alex Williamson]

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

Re: [Qemu-devel] [PATCH] CODING_STYLE: Define our preferred form for multiline comments

[Thomas Huth]

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

Re: [Qemu-devel] [PATCH] CODING_STYLE: Define our preferred form for multiline comments

[Cornelia Huck]

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.

Re: [Qemu-devel] [PATCH] CODING_STYLE: Define our preferred form for multiline comments

[Peter Maydell]

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

Re: [Qemu-devel] [PATCH] CODING_STYLE: Define our preferred form for multiline comments

[Thomas Huth]

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

Re: [Qemu-devel] [PATCH] CODING_STYLE: Define our preferred form for multiline comments

[Cornelia Huck]

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".

Re: [Qemu-devel] [PATCH] CODING_STYLE: Define our preferred form for multiline comments

[Markus Armbruster]

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.

Re: [Qemu-devel] [PATCH] CODING_STYLE: Define our preferred form for multiline comments

[Kevin Wolf]

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

Re: [Qemu-devel] [PATCH] CODING_STYLE: Define our preferred form for multiline comments

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 245 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PATCH] CODING_STYLE: Define our preferred form for multiline comments

[Peter Maydell]

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