Hi all, On Tue, 2020-10-20 at 18:13 +0100, Julien Grall wrote: > Hi, > > On 19/10/2020 19:07, Stefano Stabellini wrote: > > On Fri, 16 Oct 2020, Artem Mygaiev wrote: > > > -----Original Message----- > > > From: Julien Grall <jul...@xen.org> > > > Sent: пятница, 16 октября 2020 г. 13:24 > > > To: Anastasiia Lukianenko <anastasiia_lukiane...@epam.com>; > > > jbeul...@suse.com; george.dun...@citrix.com > > > Cc: Artem Mygaiev <artem_myga...@epam.com>; vicooo...@gmail.com; > > > xen-devel@lists.xenproject.org; committ...@xenproject.org; > > > viktor.mitin...@gmail.com; Volodymyr Babchuk < > > > volodymyr_babc...@epam.com> > > > Subject: Re: Xen Coding style and clang-format > > > > > > > Hi, > > > > > > > > On 16/10/2020 10:42, Anastasiia Lukianenko wrote: > > > > > Thanks for your advices, which helped me improve the checker. > > > > > I > > > > > understand that there are still some disagreements about the > > > > > formatting, but as I said before, the checker cannot be very > > > > > flexible > > > > > and take into account all the author's ideas. > > > > > > > > I am not sure what you refer by "author's ideas" here. The > > > > checker > > > > should follow a coding style (Xen or a modified version): > > > > - Anything not following the coding style should be > > > > considered as > > > > invalid. > > > > - Anything not written in the coding style should be left > > > > untouched/uncommented by the checker. > > > > > > > > > > Agree > > > > > > > > I suggest using the > > > > > checker not as a mandatory check, but as an indication to the > > > > > author of > > > > > possible formatting errors that he can correct or ignore. > > > > > > > > I can understand that short term we would want to make it > > > > optional so > > > > either the coding style or the checker can be tuned. But I > > > > don't think > > > > this is an ideal situation to be in long term. > > > > > > > > The goal of the checker is to automatically verify the coding > > > > style and > > > > get it consistent across Xen. If we make it optional or it is > > > > "unreliable", then we lose the two benefits and possibly > > > > increase the > > > > contributor frustration as the checker would say A but we need > > > > B. > > > > > > > > Therefore, we need to make sure the checker and the coding > > > > style match. > > > > I don't have any opinions on the approach to achieve that. > > > > > > Of the list of remaining issues from Anastasiia, looks like only > > > items 5 > > > and 6 are conform to official Xen coding style. As for remainning > > > ones, > > > I would like to suggest disabling those that are controversial > > > (items 1, > > > 2, 4, 8, 9, 10). Maybe we want to have further discussion on > > > refining > > > coding style, we can use these as starting point. If we are open > > > to > > > extending style now, I would suggest to add rules that seem to be > > > meaningful (items 3, 7) and keep them in checker. > > > > Good approach. Yes, I would like to keep 3, 7 in the checker. > > > > I would also keep 8 and add a small note to the coding style to say > > that > > comments should be aligned where possible. > > +1 for this. Although, I don't mind the coding style used as long as > we > have a checker and the code is consistent :). > > Cheers, > Thank you for advices :) Now I'm trying to figure out the option that needs to be corrected for the checker to work correctly: Wrapping an operation to a new line when the string length is longer than the allowed - status = acpi_get_table(ACPI_SIG_SPCR, 0, - (struct acpi_table_header **)&spcr); + status = + acpi_get_table(ACPI_SIG_SPCR, 0, (struct acpi_table_header **)&spcr); As it turned out, this case is quite rare and the rule for transferring parameters works correctly in other cases: - status = acpi_get_table(ACPI_SIG_SPCR, 0, &spcr, ACPI_SIG_SPC, 0, ACPI_SIG_SP, 0); + status = acpi_get_table(ACPI_SIG_SPCR, 0, &spcr, ACPI_SIG_SPC, 0, + ACPI_SIG_SP, 0); Thus the checker does not work correctly in the case when the prototype parameter starts with a parenthesis. I'm going to ask clang community is this behavior is expected or maybe it's a bug.
Regards, Anastasiia
