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

[Julien Grall <julien.grall@xxxxxxx>]

Hi Rich,

On 19/07/2019 14:50, Rich Persaud 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?
It was originally started by EPAM to address review burden. IIRC, the goal is to 
have a robot checking coding style before any review is actually done. So 
probably part of Xen's Gitlab.
I am not sure why you mention OpenEmbedded/Yocto or even EPAM's Xen. The patch 
is against clang-format (from LLVM project) and does not have any dependencies 
on all the rest.
> 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.
I will leave that to Lars and Artem. They have been following the work more 
closely.

Cheers,

--
Julien Grall

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