> On Oct 7, 2021, at 6:48 AM, Leif Lindholm <l...@nuviainc.com> 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
>>>>>
>>>>
>>>
>>>
>>>
>>>
>>>
>>
>>
>>
>>
>>
>>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#81580): https://edk2.groups.io/g/devel/message/81580
Mute This Topic: https://groups.io/mt/84932137/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
