Patch 1 was sent before, iirc without ever having heard back anything.
Patch 2 is a result of a recent observation of Tamas.

1: explicitly call out label indentation
2: list further brace placement exceptions

Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Since the behavior of "diff -p" to use an unindented label as context
identifier often makes it harder to review patches, make explicit the
requirement for labels to be indented.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/CODING_STYLE
+++ b/CODING_STYLE
@@ -31,6 +31,10 @@ void fun(void)
      }
  }
  
+Due to the behavior of GNU diffutils "diff -p", labels should be
+indented by at least one blank.  Non-case labels inside switch() bodies
+are preferred to be indented the same as the block's case labels.
+
  White space
  -----------
  

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

On Fri, 19 Jul 2019, Jan Beulich wrote:
> Since the behavior of "diff -p" to use an unindented label as context
> identifier often makes it harder to review patches, make explicit the
> requirement for labels to be indented.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Stefano Stabellini <sstabellini@kernel.org>


> --- a/CODING_STYLE
> +++ b/CODING_STYLE
> @@ -31,6 +31,10 @@ void fun(void)
>       }
>   }
>   
> +Due to the behavior of GNU diffutils "diff -p", labels should be
> +indented by at least one blank.  Non-case labels inside switch() bodies
> +are preferred to be indented the same as the block's case labels.
> +
>   White space
>   -----------
>   
> 
>

On Fri, Jul 19, 2019 at 3:19 AM Jan Beulich <JBeulich@suse.com> wrote:
>
> Since the behavior of "diff -p" to use an unindented label as context
> identifier often makes it harder to review patches, make explicit the
> requirement for labels to be indented.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

This style requirement wouldn't really work with astyle as-is.

Tamas

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

On 19.07.2019 15:18, Tamas K Lengyel wrote:
> On Fri, Jul 19, 2019 at 3:19 AM Jan Beulich <JBeulich@suse.com> wrote:
>>
>> Since the behavior of "diff -p" to use an unindented label as context
>> identifier often makes it harder to review patches, make explicit the
>> requirement for labels to be indented.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> This style requirement wouldn't really work with astyle as-is.

Personally I view proper "diff -p" context in patches as quite
a bit more important than automatic style checking. But perhaps
that's just because I do quite a lot of patch review ...

Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

On Fri, Jul 19, 2019 at 7:22 AM Jan Beulich <JBeulich@suse.com> wrote:
>
> On 19.07.2019 15:18, Tamas K Lengyel wrote:
> > On Fri, Jul 19, 2019 at 3:19 AM Jan Beulich <JBeulich@suse.com> wrote:
> >>
> >> Since the behavior of "diff -p" to use an unindented label as context
> >> identifier often makes it harder to review patches, make explicit the
> >> requirement for labels to be indented.
> >>
> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >
> > This style requirement wouldn't really work with astyle as-is.
>
> Personally I view proper "diff -p" context in patches as quite
> a bit more important than automatic style checking. But perhaps
> that's just because I do quite a lot of patch review ...

Which is a valid point. I don't really care which style option it
really is, if it helps you, I'm fine with it. But as a maintainer who
does this on a volunteer basis when I have a 5-minute window, I'm not
going to spend my time looking for style issues. So in the ongoing
vm_event series that's under review where you explicitly called out
that the vm_event maintainers have to review and point out all style
issues, that's a no-go from my side. So either we have automatic style
checks or I'm just going to Ack patches with style issues.

Tamas

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

For easy spotting of struct/union/enum definitions we already commonly
place the opening braces on the initial line of such a definition.

We also often don't place the opening brace of an initializer on a
separate line.

And finally for compound literals placing the braces on separate lines
often makes the code more difficult to read, so it should (and in
practice does) typically go on the same line as well.  The placement of
the closing brace often depends on how large such a compound literal is.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
TBD: We may want to make explicit that for initializers both forms are
      fine.

--- a/CODING_STYLE
+++ b/CODING_STYLE
@@ -64,8 +64,13 @@ Bracing
  -------
  
  Braces ('{' and '}') are usually placed on a line of their own, except
-for the do/while loop.  This is unlike the Linux coding style and
-unlike K&R.  do/while loops are an exception. e.g.:
+for
+- the do/while loop
+- the opening brace in definitions of enum, struct, and union
+- the opening brace in initializers
+- compound literals
+This is unlike the Linux coding style and unlike K&R.  do/while loops
+are one exception. e.g.:
  
  if ( condition )
  {

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

On Fri, 19 Jul 2019, Jan Beulich wrote:
> For easy spotting of struct/union/enum definitions we already commonly
> place the opening braces on the initial line of such a definition.
> 
> We also often don't place the opening brace of an initializer on a
> separate line.
> 
> And finally for compound literals placing the braces on separate lines
> often makes the code more difficult to read, so it should (and in
> practice does) typically go on the same line as well.  The placement of
> the closing brace often depends on how large such a compound literal is.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
> TBD: We may want to make explicit that for initializers both forms are
>       fine.
> 
> --- a/CODING_STYLE
> +++ b/CODING_STYLE
> @@ -64,8 +64,13 @@ Bracing
>   -------
>   
>   Braces ('{' and '}') are usually placed on a line of their own, except
> -for the do/while loop.  This is unlike the Linux coding style and
> -unlike K&R.  do/while loops are an exception. e.g.:
> +for
> +- the do/while loop
> +- the opening brace in definitions of enum, struct, and union
> +- the opening brace in initializers
> +- compound literals
> +This is unlike the Linux coding style and unlike K&R.  do/while loops
> +are one exception. e.g.:
>   
>   if ( condition )
>   {
> 
>