Re: [Xen-devel] [RFC] xen: Add .astylerc for automated style-formatting

[Viktor Mitin <viktor.mitin.19@xxxxxxxxx>]

Hi Julien, All,

On Thu, Jul 18, 2019 at 6:44 PM Julien Grall <julien.grall@xxxxxxx> wrote:
>
> Hi Tamas,
>
> On 7/18/19 4:14 PM, Tamas K Lengyel wrote:
> > On Thu, Jul 18, 2019 at 9:02 AM Julien Grall <julien.grall@xxxxxxx> wrote:
> >>
> >> Hi Tamas,
> >>
> >> Adding Lars, Artem and Iurii. Iurii has been working on a version for
> >> clang-format recently.
> >>
> >> On 7/18/19 3:43 PM, Tamas K Lengyel wrote:
> >>> Using astyle (http://astyle.sourceforge.net) can greatly reduce the 
> >>> overhead of
> >>> manually checking and applying style-fixes to source-code. The included
> >>> .astylerc is the closest approximation of the established Xen style 
> >>> (including
> >>> styles not formally spelled out by CODING_STYLE but commonly requested).
> >>>
> >>> Checking the comment styles are not included in the automation.
> >>>
> >>> Incorporating Xen's exception to the do-while style is only partially 
> >>> possible,
> >>> thus a change is proposed to the CODING_STYLE of moving the brace from 
> >>> "do {"
> >>> to the next line.
> >>>
> >>> Most of Xen's code-base is non-conforming at the moment: 289 files pass
> >>> unchanged, 876 have some style issue
> >>>
> >>> Ideally we can slowly migrate the entire code-base to be conforming, thus
> >>> eliminating the need of discussing and enforcing style issues manually on 
> >>> the
> >>> mailinglist.
> >>
> >> I quite like the idea of an automatic coding style checker. However, it
> >> is a bit concerning that not even a 1/3 of the files are able to pass
> >> the coding style you suggest. Could you explain whether this is because
> >> the files does not already follow Xen coding style or is it just the
> >> difference with astyle?
> >>
> >> What are the main style issues?
> >
> > Looks like a lot of files that don't follow the Xen coding style
> > as-is. Alignment issues seem to me to be the most common errors. See
> > the full diff here:
> >
> > https://gist.github.com/tklengyel/c5cac14a0d57f119dd7747a1be6fb260
> >
> > We can perhaps tune some aspects of it we disagree with the astyle
> > generated style and try to override it. I did my best to make it
> > conform to the existing Xen style but certainly there could be other
> > tweaks made to reduce the churn.
>
> I think we definitely want to avoid churn as this is going to take a lot
> of time to fix all the places to the new indentation.
>
> Going through the diff I can see major differences with the Xen Coding
> style and also what looks like inconsistencies from the tools itself:
>    - Line 58: This is fairly common to indent the parameters as it is
> today. But then on line 158/272 it indents as we do today. So I am not
> sure what the expected coding style from the tools.
>    - Line 67: I believe Jan request the space before label
Seems agreed not to add the spaces before label. Right?

>    - Line 139: The { are commonly on the same line as struct or definition.
This should be stated in the Coding style explicitly.

>    - Line 276: The switch case indentation was correct from Xen PoV before
>    - Line 289: Files imported from Linux should not be touch here.

The code files to style mapping to be automated outside of clang-format tool.
Clang-format tool supports  3 new formatting styles to cover all the
cases mentioned in Xen coding style document: Xen, Libxl,Linux

>    - Line 1025: The tools needs to be able to deal for_each_vcpu(...) & co.
Clang-format tool supports such cases.

>    - Line 8735: It looks like to me the tools policy is quite
> inconsistent. In previous place it keeps it properly aligned see line 5777.
>
> I have only looked quickly through the diff, but I think they are the
> main one that should probably be resolved.
>

Please be aware that it is important to add all the cases mentioned
above (and all the other) to the Xen Coding style document explicitly.
This seems the biggest non-technical issue to overcome...

> Cheers,
>
> --
> Julien Grall
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/mailman/listinfo/xen-devel

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