[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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
  - Line 139: The { are commonly on the same line as struct or definition.
  - Line 276: The switch case indentation was correct from Xen PoV before
  - Line 289: Files imported from Linux should not be touch here.
  - Line 1025: The tools needs to be able to deal for_each_vcpu(...) & co.
- 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.

Cheers,

--
Julien Grall

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.