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

[Jan Beulich <JBeulich@xxxxxxxx>]

On 29.07.2019 14:49, Julien Grall wrote:
> 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.

I agree with what you say in general, but not in this specific case: I've
explained why the leading blank(s) are wanted here. This is not because of
my taste, but because of helping with patch review.

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