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

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



On Fri, Jul 19, 2019 at 7:31 AM 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
>

Thanks, I'll give this a shot. Since this requires patching clang it
looks to me like it may be a while before it's generally available
downstream. Perhaps it's still worth exploring adding .astylerc and
have at least partial coverage for the automated style checks for the
interim.

Tamas

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