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

[Lars Kurth <lars.kurth@xxxxxxxxxx>]

On 19/07/2019, 14:50, "Rich Persaud" <persaur@xxxxxxxxx> wrote:

    On Jul 19, 2019, at 09:31, Julien Grall <julien.grall@xxxxxxx> wrote:
    >> On 19/07/2019 14:24, Julien Grall wrote:
    >> Hi Tamas,
    >>> On 19/07/2019 14:14, Tamas K Lengyel wrote:
    >>>> On Fri, Jul 19, 2019 at 7:11 AM Julien Grall <julien.grall@xxxxxxx> 
wrote:
    >>>> 
    >>>> Hi Tamas,
    >>>> 
    >>>>> On 19/07/2019 14:00, Tamas K Lengyel wrote:
    >>>>>> On Fri, Jul 19, 2019 at 2:43 AM Julien Grall <julien.grall@xxxxxxx> 
wrote:
    >>>>>> 
    >>>>>> Hi Tamas,
    >>>>>> 
    >>>>>> On 18/07/2019 18:48, Tamas K Lengyel wrote:
    >>>>>>>>       - Line 1025: The tools needs to be able to deal 
for_each_vcpu(...) & co.
    >>>>>>> 
    >>>>>>> These can be made OK by adding braces. Other than that the only way 
I
    >>>>>>> found to make it not change the indentation is to add the comment 
"/*
    >>>>>>> *INDENT-OFF* */" before the block and "/* *INDENT-ON* */" 
afterwards.
    >>>>>> 
    >>>>>> None of them looks really appealing because it means astyle will not 
correctly
    >>>>>> indent if the user does not add braces or comments.
    >>>>>> 
    >>>>>> Could astyle be easily modified to recognize foreach macros?
    >>>>> 
    >>>>> Not that I'm aware of. If you don't want to manually annotate files
    >>>>> with unsupported macros then just exclude those files from astyle. I
    >>>>> wouldn't recommend adding this to the CI for all files, only for those
    >>>>> that their respective maintainers have confirmed to conform to the
    >>>>> style and want to enforce it going forward.
    >>>> 
    >>>> So a couple use of an unsupported macros would make impossible to 
enforce the
    >>>> coding style. This is not a very ideal position to be in.
    >>>> 
    >>>> _if_ we are going to adopt astyle then we need to be able to enforce 
it on every
    >>>> Xen files long-term. If it is not possible to do it with astyle, then 
maybe this
    >>>> is not the right tool to use.
    >>>> 
    >>>> For instance, I know that tools such as clang-format is able to deal 
with
    >>>> foreach macros.
    >>> 
    >>> If there are better tools then sure, I don't really mind using
    >>> something else. I just don't have time to do the manual style check
    >>> back-and-forth anymore, so the sooner we have something in place the
    >>> better.
    >> I totally agree we need a tool so the reviewer can free-up some time to 
focus on more important things. However, I think we should be careful on what 
we adopt here.
    >> Similar to Andrew, I am open with modifying the coding style to help the 
automatic style check. But I am not happy to disable automatic style on part 
(or entire) of files forever.
    >> At the moment, clang-format feels more powerful and there are people 
working on it.
    > 
    > FYI, below a link to the clang-format changes:
    > 
    > 
https://github.com/xen-troops/Xen-Clang-format/blob/devel/clang-format.patch
    
    Were these clang-format changes done for FuSa work?  Are they intended to 
be run within OSStest and/or Xen's Gitlab CI, which do not currently support 
OpenEmbedded/Yocto and xen-troops?

No, they were done following a discussion at a previous summit and then nothing 
moved for two years
The idea was to avoid reviewers having to do mechanical tasks such as review 
styling
    
    It would be helpful to have a xen-devel thread on the motivation for the 
clang-format work, the specific style being enforced (including the nuances 
discussed in this thread) and additional work needed before clang-format can 
perform automated style checking to address (a) existing Xen/Linux style 
requirements, (b) FuSa requirements.

Ultimately, we want to integrate this into the CIv2 we discussed and agreed at 
the summit. Aka an automatic style check triggered by a mailing list post 
alongside build checks and basic smoke tests on top of QEMU. I still need to 
get on top of the notes

Lars
P.S.: Sorry, catching up on mailing list and stuff after a 2 week trip

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