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

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



Hi Jan,

On 7/29/19 1:19 PM, Jan Beulich wrote:
On 26.07.2019 16:49, Viktor Mitin wrote:
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?

Certainly not, afaia. I will object to any written down rule disallowing
leading blank(s) altogether. I will argue for making mandatory at least
one blank of indentation.

Coding style are a matter of taste. If everyone is going to say "I want this in the coding style", then we are going to spend countless of hours bike-shedding. This is not how we should use our already limited time.

If we want to re-use existing checker, then we will most likely have to make some compromise. I am not suggesting this should be (or not be) part of the compromises.

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