Re: [edk2-devel] Uncrustify Conversion Detailed Plan and Extended Hard Freeze Update #4

["Michael D Kinney" <michael.d.kinney@intel.com>]

Michael,

Given that there are now 2 requests for this feature, I think it is a
good idea to add the feature.

Package YAML files are part of the review content, so we can make sure 
we all look at these YAML changes and provide feedback when this
uncrustify exclusion feature is applied in areas that do not make
sense.

One additional challenge with the exclusion feature is how developers
can run uncrustify locally to prepare their patch reviews.  They can
not run it on all c/h files in a package.  Now they have to know which
directories to exclude.  This applies to running uncrustify from command
line and usages such as Visual Studio Code plugin.  In order to make 
it easy for developers to always get the right format, you would need
a helper tool that can look at the YAML file to know if there are
areas that are excluded and exclude files from the uncrustify processing.
I am concerned that someone will miss the exclusion list in patch 
generation and review and the uncrustified versions will get checked 
in by accident at some point.  EDK II CI will not care because it will
skip the UncrustifyCheck on excluded files/dirs.

Thanks,

Mike

> -----Original Message-----
> From: Michael Kubacki <mikuback@linux.microsoft.com>
> Sent: Thursday, December 2, 2021 9:15 AM
> To: Gerd Hoffmann <kraxel@redhat.com>
> Cc: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>; Chang, Abner <abner.chang@hpe.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Michael Kubacki <michael.kubacki@microsoft.com>; Andrew Fish (afish@apple.com) <afish@apple.com>;
> Leif Lindholm <leif@nuviainc.com>
> Subject: Re: [edk2-devel] Uncrustify Conversion Detailed Plan and Extended Hard Freeze Update #4
> 
> Thank you for clarifying. This case is more reasonable to me since the
> files are auto generated and not ported into edk2.
> 
> There is an option to bring files like this in as a new submodule but I
> understand why that might not be desirable and would be disruptive to
> the current process.
> 
> If there's no objection, I'll add an option to exclude paths within a
> package in a v6 patch of the plugin and leave this to maintainer
> discretion. I just ask that the spirit of consistent formatting be
> considered when making these decisions.
> 
> Regards,
> Michael
> 
> On 12/2/2021 11:23 AM, Gerd Hoffmann wrote:
> > On Thu, Dec 02, 2021 at 10:33:14AM -0500, Michael Kubacki wrote:
> >> Hi Gerd,
> >>
> >> To help me understand which files you're specifically referring to, can you
> >> please point them out from this commit? Or provide additional details?
> >>
> >> https://github.com/tianocore/edk2/pull/2229/commits/50654dfe5785964c9ae72961d13a50b26af77794
> >>
> >> CryptoPkg/Library/Include/openssl/opensslconf.h is currently formatted.
> >
> > Yes, that one, and there is another in CryptoPkg/Library/Include/crypto/
> > The switch to openssl 3.0 will add more of those files[1]
> >
> > They are generated from openssl source code,
> > CryptoPkg/Library/OpensslLib/process_files.pl handles that.
> >
> > I think the two reasonable options to deal with that are:
> >
> >    (1) exclude those files from formating, or
> >    (2) call uncrustify in process_files.pl to reformat them
> >        each time they are generated.
> >
> > Given that these files will never be edited manually my personal
> > preference would be (1).
> >
> > take care,
> >    Gerd
> >
> > [1] wip branch @ https://github.com/kraxel/edk2/commits/openssl3
> >