> On Oct 7, 2021, at 6:48 AM, Leif Lindholm wrote: > > Hi Michael, > > Apologies, I've owed you a response (promised off-list) for a while > now. > > First, let me say I hugely appreciate this effort. Apart from aligning > the codebase(s), this will reduce manual reviewing effort > substantially, as well as cutting down on number of rework cycles for > developers. > > Looking at the changes to (well, the comments in) uncrustify, this > seems to be constrained to: > - Newline after '(' for multi-line function calls. > - Dealing with "(("/"))" for DEBUG macros. > - Function pointer typedefs: > - typedef\nEFIAPI > - closing parentheses indentation > > I don't think I've made any secret over the years that I am not a > massive fan of the EDK2 coding style in general. So I think for any > of its quirks that are substantial enough that they require not just > custom configuration but actual new function added to existing code > conformance tools, this would be an excellent point to sanitise the > coding style instead. > > Taking these in order: > > Newline after '(' > ----------------- > I think we already reached a level of flexibility around this, where > we don't actually enforce this (or single argument per > line). Personally, I'd be happy to update the coding style as > required instead. > > DEBUG macro parentheses > ----------------------- > How does uncrustify treat DEBUG macros without this modification? > Do we start getting everything turned into multi-level indented > multi-line statements without this change? Can we disable the rule that does not like the DEBUG macro? I seem to remember clang nags about some strange () usage, so maybe we would not lose that much? Thanks, Andrew Fish > > Function pointer typedefs: > -------------------------- > I don't see that function pointer typedefs need to rigidly follow the > same pattern as the declaration of functions implementing them. Could > we update the coding style (if needed) instead? > > Best Regards, > > Leif > > On Mon, Aug 16, 2021 at 16:00:38 -0400, Michael Kubacki wrote: >> The edk2 branch was created here: >> https://github.com/makubacki/edk2/tree/uncrustify_poc_2 >> >> We have a Project Mu fork with custom changes to the Uncrustify tool to help >> comply with EDK II formatting here: >> https://dev.azure.com/projectmu/_git/Uncrustify >> >> The latest information about the status and how to experiment with the >> configuration file and the tool are in that fork: https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Project-Mu-(EDK-II)-Fork-Readme >> >> That said, I have also finished a CI plugin to run Uncrustify that should be >> ready soon to initially deploy in Project Mu. Before doing so, I am trying >> to settle on an initial configuration file that less strictly but more >> reliably formats the code than in the examples in those branches. For >> example, remove heuristics that when run against the same set of code >> multiple times can produce different results. An example would be a rule >> that reformats code because it exceeds a specified column width on one run >> but on the next run that reformatted code triggers a different rule to >> further align the code and so on. At least initially, some rules might be >> tweaked in a more conservative approach that can be tightened in the future. >> Once this configuration file is ready, we will baseline Project Mu code as >> an example and turn on the plugin. The CI plugin runs Uncrustify against >> modified files and if there's any changes, indicating a formatting >> deviation, the diff chunks are saved in a log so they can be viewed as a >> build artifact. >> >> I am making progress on the updated config file and I should be able to post >> a "uncrustify_poc_3" branch soon with the results. >> >> Regarding indentation, Marvin is right that Uncrustify cannot support edk2 >> indentation style out-of-box. Some changes are made in that fork to handle >> the formatting. At this point, it can handle the indentation in the cases >> I've seen. Uncrustify does potentially give us the ability to massively >> deploy changes across the codebase in case a decision were made to change >> the style. >> >> Thanks, >> Michael >> >> On 8/16/2021 3:39 PM, Marvin Häuser wrote: >>> Hey Rebecca, >>> >>> I think even Uncrustify has issues with the EDK II indentation style. >>> You might want to check the UEFI Talkbox Discord server, I had a brief >>> chat with Michael about it there. I don't think realistically any tool >>> supports EDK II's indentation style however, so I'd propose it is >>> changed. This could be for new submissions only, or actually the entire >>> codebase could be reformatted at once with a good tool setup. While this >>> screws with git blame, the (to my understanding) decided on CRLF -> LF >>> change does that anyway, so at least two evils could be dealt with in >>> one go really. >>> >>> Best regards, >>> Marvin >>> >>> On 16/08/2021 21:34, Rebecca Cran wrote: >>>> >>>> cc devel@ . >>>> >>>> On 8/16/21 1:33 PM, Rebecca Cran wrote: >>>>> >>>>> I noticed a message on Twitter about an idea of using Uncrustify >>>>> for EDK2 instead of the ECC tool, and came across https://www.mail-archive.com/search?l=devel@edk2.groups.io&q=subject:%22Re%5C%3A+%5C%5Bedk2%5C-devel%5C%5D+TianoCore+Community+Meeting+Minutes+%5C-+2%5C%2F4%22&o=newest&f=1 >>>>> . >>>>> >>>>> I was wondering if there's been any progress on it that I could >>>>> check out? >>>>> >>>>> >>>>> Michael Kubacki: in that message, you said: >>>>> >>>>> "I'm planning to put up a branch that we can use as a reference >>>>> for a conversation around uncrustify in the next couple of >>>>> weeks." >>>>> >>>>> >>>>> Did you end up creating that branch, and if so could you provide >>>>> a link to it please? >>>>> >>>>> >>>>> -- >>>>> Rebecca Cran >>>>> >>>> >>> >>> >>> >>> >>> >> >> >> >> >> >> > > >