public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Wang, Jian J" <jian.j.wang@intel.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
	"Kinney, Michael D" <michael.d.kinney@intel.com>,
	Michael Kubacki <mikuback@linux.microsoft.com>,
	"maciej.rabeda@linux.intel.com" <maciej.rabeda@linux.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
Date: Tue, 7 Dec 2021 03:47:32 +0000	[thread overview]
Message-ID: <CO1PR11MB4945272431D6C62155BEDD4BB66E9@CO1PR11MB4945.namprd11.prod.outlook.com> (raw)
In-Reply-To: <CO1PR11MB49292C68B52F6D60BE2B4B83D26D9@CO1PR11MB4929.namprd11.prod.outlook.com>

For CryptoPkg, SecurityPkg and SignedCapsulePkg,

	Reviewed-by: Jian J Wang <jian.j.wang@intel.com>

Regards,
Jian

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael D
> Kinney
> Sent: Monday, December 06, 2021 9:18 AM
> To: Michael Kubacki <mikuback@linux.microsoft.com>; devel@edk2.groups.io;
> maciej.rabeda@linux.intel.com; Michael Kubacki
> <michael.kubacki@microsoft.com>; Andrew Fish (afish@apple.com)
> <afish@apple.com>; Leif Lindholm <leif@nuviainc.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: Re: [edk2-devel] Uncrustify Conversion Detailed Plan and Extended
> Hard Freeze Update #4
> 
> Hello EDK II Maintainers,
> 
> A detailed evaluation of the DEBUG_CODE() formatting issue has been
> completed.
> The reason DEBUG_CODE() is a challenge is that this looks like a macro from a
> C parsing perspective, but the EDK II usage places C statements or blocks of
> C code as the parameter to this macro.
> 
> There are actually 2 methods available to mark a statement or a block of code
> to be included when the DEBUG_PROPERTY_DEBUG_CODE_ENABLED bit is
> enabled in
> PcdDebugPropertyMask. One is DEBUG_CODE() and the other is to mark the
> beginning
> and end of a code block with DEBUG_CODE_BEGIN() and DEBUG_CODE_END().
> In fact,
> DEBUG_CODE() is implemented using DEBUG_CODE_BEGIN() and
> DEBUG_CODE_END() macros.
> 
>     #define DEBUG_CODE(Expression)  \
>       DEBUG_CODE_BEGIN ();          \
>       Expression                    \
>       DEBUG_CODE_END ()
> 
> A complete review for the use of these DEBUG_CODE macros was performed on
> the
> edk2 repo.  Uncrustify performs good formatting for code blocks between
> DEBUG_CODE_BEGIN() and DEBUG_CODE_END().  This is because these look
> like simple
> macros calls with no parameters and the lines of C code between these 2
> macros
> is formatted correctly.
> 
> The uncrustify formatting issues are only present with the use of DEBUG_CODE().
> Simple use cases of DEBUG_CODE(Expression) where Expression is a single C
> statement also look correct.  A medium complexity use case where Expression is
> a code block of simple statements or even some local variables and simple
> statements  also look correct.  It is only complex code blocks that use C
> statements such as if/for/while/case that include the use of braces {} does
> uncrustify perform incorrect formatting.
> 
> The recommended solution to this issue is to convert the use of DEBUG_CODE()
> to DEBUG_CODE_BEGIN() / DEBUG_CODE_END() for cases where the
> Expression
> passed to DEBUG_CODE() is the complex use case that contains statements that
> use braces {}.  There are 57 instances of this pattern across 40 files in the
> edk2 repo.
> 
> I have posted a branch with these additional patches:
> 
> 
> https://github.com/mdkinney/edk2/tree/Bug_3737_3760_3739_ApplyUncrustif
> yChanges_V7
> 
> I have performed CompareBuild tests with this revised patch series with
> the DEBUG_CODE changes.  It passes 100% showing no binary differences.
> 
>     https://github.com/mdkinney/edk2/actions/runs/1542454606
> 
> I have opened a PR to run this patch series through EDK II CI. It also passes 100%.
> 
>     https://github.com/tianocore/edk2/pull/2236
> 
> The summary of changes made since the V6 review are:
> 
>     1) Change uncrustify configuration assignment alignment threshold to 0
> 
>         align_assign_thresh = 0
> 
>     2) Replace "<param>, OPTIONAL" with "<param> OPTIONAL,"
> 
>     3) Replace DEBUG_CODE(Expression) with
> 
>            DEBUG_CODE_BEGIN();
>            Expression
>            DEBUG_CODE_END()
> 
>        if Expression is complex and contains braces {}.
> 
>     4) No changes to uncrustify tool required.
> 
> Please review the differences between the following 2 branches and provide
> feedback or a Series Reviewed-by if you agree with these additional changes.
> 
> 
> https://github.com/mdkinney/edk2/tree/Bug_3737_3760_3739_ApplyUncrustif
> yChanges_V6
> 
> https://github.com/mdkinney/edk2/tree/Bug_3737_3760_3739_ApplyUncrustif
> yChanges_V7
> 
> The goal is to complete the review and get the uncrustify change committed
> tomorrow so the extended hard freeze can be lifted.
> 
> Thanks,
> 
> Mike
> 
> > -----Original Message-----
> > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > Sent: Thursday, December 2, 2021 6:23 PM
> > To: Michael Kubacki <mikuback@linux.microsoft.com>; devel@edk2.groups.io;
> maciej.rabeda@linux.intel.com; Michael Kubacki
> > <michael.kubacki@microsoft.com>; Andrew Fish (afish@apple.com)
> <afish@apple.com>; Leif Lindholm <leif@nuviainc.com>;
> > Kinney, Michael D <michael.d.kinney@intel.com>
> > Subject: RE: [edk2-devel] Uncrustify Conversion Detailed Plan and Extended
> Hard Freeze Update #4
> >
> > Hello EDK II Maintainers,
> >
> > I have entered BZ 3760 to make the use of the OPTIONAL keyword style
> consistent for all of edk2 repo
> > and to be compatible with uncrustify.
> >
> > I have posted the following V6 branch that does the EFI_D_* to DEBUG_*
> changes, the OPTIONAL keyword
> > style changes, and the uncrustify changes with the one configuration change
> for assignment alignment.
> >
> >
> https://github.com/mdkinney/edk2/tree/Bug_3737_3760_3739_ApplyUncrustif
> yChanges_V6
> >
> > Please provide feedback on the code style in this branch with the known
> DEBUG_CODE() issue still
> > present.
> >
> > If we are able to quickly update uncrustify to handle DEBUG_CODE(), I will
> generate a V7.
> >
> > Thanks,
> >
> > Mike
> >
> > > -----Original Message-----
> > > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > > Sent: Thursday, December 2, 2021 4:53 PM
> > > To: Michael Kubacki <mikuback@linux.microsoft.com>;
> devel@edk2.groups.io; maciej.rabeda@linux.intel.com; Michael Kubacki
> > > <michael.kubacki@microsoft.com>; Andrew Fish (afish@apple.com)
> <afish@apple.com>; Leif Lindholm <leif@nuviainc.com>;
> > > Kinney, Michael D <michael.d.kinney@intel.com>
> > > Subject: RE: [edk2-devel] Uncrustify Conversion Detailed Plan and Extended
> Hard Freeze Update #4
> > >
> > > Michael,
> > >
> > > Yes.  Please update the patch series that adds UncrustifyCheck with those
> changes.
> > >
> > > Thanks,
> > >
> > > Mike
> > >
> > > > -----Original Message-----
> > > > From: Michael Kubacki <mikuback@linux.microsoft.com>
> > > > Sent: Thursday, December 2, 2021 4:31 PM
> > > > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> devel@edk2.groups.io; maciej.rabeda@linux.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
> > > >
> > > > Hi Mike,
> > > >
> > > > Thank you for the detailed analysis and recommendations.
> > > >
> > > > I agree with the recommendations and I will try to have an Uncrustify
> > > > tool update by tomorrow for (4).
> > > >
> > > > That will require an update in uncrustify_ext_dep.yaml to pull in the
> > > > new version. I'm assuming I should also update uncrustify.cfg to set
> > > > align_assign_thresh = 0 in that new patch series?
> > > >
> > > > Regards,
> > > > Michael
> > > >
> > > > On 12/2/2021 7:18 PM, Kinney, Michael D wrote:
> > > > > Hi Michael,
> > > > >
> > > > > CORRECTION: set align_assign_threshold to 0.
> > > > >
> > > > > Reponses inline below.
> > > > >
> > > > > I would like to summarize the 4 issues raised in the past day along with
> the recommendations.
> > > > >
> > > > > 1) Exclusion feature for UncrustifyCheck.  There are 2 directories with 8
> files total that
> > > > >     maintainers have noted they would like to see not go through
> uncrustify formatting.  Today
> > > > >     the only content that is skipped is BaseTools and submodules.
> > > > >
> > > > >     Adding a general purpose exclusion feature would then require all
> developers to make
> > > > >     sure their method of using uncrustify also excludes those same areas.
> This requires
> > > > >     extra steps for all developers and maintainers.
> > > > >
> > > > >     If we do not add the exclusion feature, then the 8 files will require an
> extra step
> > > > >     to sync with the original source of those files.  The rate of changes of
> these 8 files
> > > > >     is very low today.
> > > > >
> > > > >     RECOMMENDATION: Do not add exclusion feature at this time.  Revisit
> if the extra work
> > > > >     to maintain the files that would be candidates for exclusions increases
> significantly.
> > > > >
> > > > > 2) Alignment of assignments.  The threshold of 4 characters appears to
> be too low and causes
> > > > >     source files that are already aligned to become unaligned.
> > > > >
> > > > >     RECOMMENDATION: Change threshold to the default value of 0 which
> means no limit.
> > > > >
> > > > >         align_assign_thresh= 0
> > > > >
> > > > > 3) Alignment of parameters in function declaration not correct.  The root
> cause of this
> > > > >     is the use of the OPTIONAL keyword.  If the OPTIONAL keyword is
> removed, then the
> > > > >     alignment is correct.  The alignment is also correct if the OPTIONAL
> keyword appears
> > > > >     before the ','.  If the OPTIONAL keyword appears after the ',', then the
> format is
> > > > >     not correct.  The OPTIONAL keyword indicates that the parameter in
> the function is
> > > > >     not required and may be passed in as NULL or 0 or some other default
> value defined by
> > > > >     the API.  It makes more sense for this OPTIONAL keyword that follows
> the parameter
> > > > >     names to appear before the ',' so it is scoped to the parameter on that
> line.  If it
> > > > >     appears after the ',', then C parsers thinks it is a prefix (IN, OUT, CONST,
> volatile,
> > > > >     static) for the next parameter in the function.
> > > > >
> > > > >     RECOMMENDATION: Update patch series with a global search and
> replace so OPTIONAL
> > > > >     keyword always appears before the ',' on the same line.
> > > > >
> > > > > 	RegEx search string:  ',( *)OPTIONAL( *)'
> > > > > 	RegEx replace string: ' $1OPTIONAL,$2'
> > > > >
> > > > > 4) Format issues with complex blocks in DEBUG_CODE(), or between
> DEBUG_CODE_BEGIN() and
> > > > >     DEBUG_CODE_END().  Uncrustify treats these as macros and is not
> aware that the
> > > > >     parameter passed into the macro call is a block of C code that needs to
> be formatted.
> > > > >     Complex blocks with if/while/for/case statements are impacted the
> most.
> > > > >
> > > > >     RECOMMENDATION: Update the uncrustify with an edk2 specific
> extension to treat these
> > > > >     macros as a block of code as if they were surrounded by an extra set of
> braces {}.
> > > > >
> > > > >
> > > > > I have posted a branch for testing purposes that implements (2) and (3).
> > > > >
> > > > > Branch:
> https://github.com/mdkinney/edk2/tree/TestOnly_Bug_3737_3739_ApplyUncru
> stifyChanges_V6_OPTIONAL_Keyword_Fix
> > > > > PR: https://github.com/tianocore/edk2/pull/2233
> > > > >    Status: PASS
> > > > > CompareBuild:
> https://github.com/mdkinney/edk2/actions/runs/1532855914
> > > > >    Status: PASS
> > > > >
> > > > > You can see what changed by fetching and comparing the following 2
> branches:
> > > > >
> > > > >
> https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrustifyChan
> ges_V5
> > > > >
> https://github.com/mdkinney/edk2/tree/TestOnly_Bug_3737_3739_ApplyUncru
> stifyChanges_V6_OPTIONAL_Keyword_Fix
> > > > >
> > > > > Please provide feedback on the RECOMMENDATIONS above.  I will go
> ahead and prepare of V6 version of
> > > > > the patch series now that that test results are all PASS.
> > > > >
> > > > > Best regards,
> > > > >
> > > > > Mike
> > > > >
> > > > >
> > > > >> -----Original Message-----
> > > > >> From: Kinney, Michael D <michael.d.kinney@intel.com>
> > > > >> Sent: Thursday, December 2, 2021 4:15 PM
> > > > >> To: Michael Kubacki <mikuback@linux.microsoft.com>;
> devel@edk2.groups.io; maciej.rabeda@linux.intel.com; Michael
> > > > Kubacki
> > > > >> <michael.kubacki@microsoft.com>; Andrew Fish (afish@apple.com)
> <afish@apple.com>; Leif Lindholm
> > <leif@nuviainc.com>;
> > > > >> Kinney, Michael D <michael.d.kinney@intel.com>
> > > > >> Subject: RE: [edk2-devel] Uncrustify Conversion Detailed Plan and
> Extended Hard Freeze Update #4
> > > > >>
> > > > >> Hi Michael,
> > > > >>
> > > > >> Reponses inline below.
> > > > >>
> > > > >> I would like to summarize the 4 issues raised in the past day along with
> the recommendations.
> > > > >>
> > > > >> 1) Exclusion feature for UncrustifyCheck.  There are 2 directories with 8
> files total that
> > > > >>     maintainers have noted they would like to see not go through
> uncrustify formatting.  Today
> > > > >>     the only content that is skipped is BaseTools and submodules.
> > > > >>
> > > > >>     Adding a general purpose exclusion feature would then require all
> developers to make
> > > > >>     sure their method of using uncrustify also excludes those same areas.
> This requires
> > > > >>     extra steps for all developers and maintainers.
> > > > >>
> > > > >>     If we do not add the exclusion feature, then the 8 files will require an
> extra step
> > > > >>     to sync with the original source of those files.  The rate of changes of
> these 8 files
> > > > >>     is very low today.
> > > > >>
> > > > >>     RECOMMENDATION: Do not add exclusion feature at this time.
> Revisit if the extra work
> > > > >>     to maintain the files that would be candidates for exclusions increases
> significantly.
> > > > >>
> > > > >> 2) Alignment of assignments.  The threshold of 4 characters appears to
> be too low and causes
> > > > >>     source files that are already aligned to become unaligned.
> > > > >>
> > > > >>     RECOMMENDATION: Change threshold to the default value of 0
> which means no limit.
> > > > >>
> > > > >>         align_assign_thresh= 4
> > > > >>
> > > > >> 3) Alignment of parameters in function declaration not correct.  The
> root cause of this
> > > > >>     is the use of the OPTIONAL keyword.  If the OPTIONAL keyword is
> removed, then the
> > > > >>     alignment is correct.  The alignment is also correct if the OPTIONAL
> keyword appears
> > > > >>     before the ','.  If the OPTIONAL keyword appears after the ',', then the
> format is
> > > > >>     not correct.  The OPTIONAL keyword indicates that the parameter in
> the function is
> > > > >>     not required and may be passed in as NULL or 0 or some other default
> value defined by
> > > > >>     the API.  It makes more sense for this OPTIONAL keyword that follows
> the parameter
> > > > >>     names to appear before the ',' so it is scoped to the parameter on
> that line.  If it
> > > > >>     appears after the ',', then C parsers thinks it is a prefix (IN, OUT,
> CONST, volatile,
> > > > >>     static) for the next parameter in the function.
> > > > >>
> > > > >>     RECOMMENDATION: Update patch series with a global search and
> replace so OPTIONAL
> > > > >>     keyword always appears before the ',' on the same line.
> > > > >>
> > > > >> 	RegEx search string:  ',( *)OPTIONAL( *)'
> > > > >> 	RegEx replace string: ' $1OPTIONAL,$2'
> > > > >>
> > > > >> 4) Format issues with complex blocks in DEBUG_CODE(), or between
> DEBUG_CODE_BEGIN() and
> > > > >>     DEBUG_CODE_END().  Uncrustify treats these as macros and is not
> aware that the
> > > > >>     parameter passed into the macro call is a block of C code that needs
> to be formatted.
> > > > >>     Complex blocks with if/while/for/case statements are impacted the
> most.
> > > > >>
> > > > >>     RECOMMENDATION: Update the uncrustify with an edk2 specific
> extension to treat these
> > > > >>     macros as a block of code as if they were surrounded by an extra set
> of braces {}.
> > > > >>
> > > > >>
> > > > >> I have posted a branch for testing purposes that implements (2) and (3).
> > > > >>
> > > > >> Branch:
> https://github.com/mdkinney/edk2/tree/TestOnly_Bug_3737_3739_ApplyUncru
> stifyChanges_V6_OPTIONAL_Keyword_Fix
> > > > >> PR: https://github.com/tianocore/edk2/pull/2233
> > > > >>    Status: PASS
> > > > >> CompareBuild:
> https://github.com/mdkinney/edk2/actions/runs/1532855914
> > > > >>    Status: PASS
> > > > >>
> > > > >> You can see what changed by fetching and comparing the following 2
> branches:
> > > > >>
> > > > >>
> https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrustifyChan
> ges_V5
> > > > >>
> https://github.com/mdkinney/edk2/tree/TestOnly_Bug_3737_3739_ApplyUncru
> stifyChanges_V6_OPTIONAL_Keyword_Fix
> > > > >>
> > > > >> Please provide feedback on the RECOMMENDATIONS above.  I will go
> ahead and prepare of V6 version of
> > > > >> the patch series now that that test results are all PASS.
> > > > >>
> > > > >> Best regards,
> > > > >>
> > > > >> Mike
> > > > >>
> > > > >>
> > > > >>> -----Original Message-----
> > > > >>> From: Michael Kubacki <mikuback@linux.microsoft.com>
> > > > >>> Sent: Thursday, December 2, 2021 1:57 PM
> > > > >>> To: devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>; maciej.rabeda@linux.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
> > > > >>>
> > > > >>> My reply is inline.
> > > > >>>
> > > > >>> Regards,
> > > > >>> Michael
> > > > >>>
> > > > >>> On 12/2/2021 2:45 PM, Michael D Kinney wrote:
> > > > >>>> Hi Maciej,
> > > > >>>>
> > > > >>>> Thanks for the feedback.
> > > > >>>>
> > > > >>>>    * Example #1.This appears to be caused by the following uncrustify
> > > > >>>>      settings:
> > > > >>>>
> > > > >>>> # The threshold for aligning on '=' in assignments.
> > > > >>>>
> > > > >>>> # Use a negative number for absolute thresholds.
> > > > >>>>
> > > > >>>> #
> > > > >>>>
> > > > >>>> # 0: No limit (default).
> > > > >>>>
> > > > >>>> align_assign_thresh= 0# number
> > > > >>>>
> > > > >>>> The edk2 setting for this is:
> > > > >>>>
> > > > >>>> align_assign_thresh= 4
> > > > >>>>
> > > > >>>> This means blocks of assignments that are different than more than 4
> > > > >>>> spaces will be considered a new block.
> > > > >>>>
> > > > >>>> ‘HwAddreLen’ and ‘Xid’ are more than 4 characters in length
> > > > >>>> different.Same for ‘Xid’ and ‘Reserved’.So
> > > > >>>>
> > > > >>>> uncrustify treats these as 3 different assignment blocks.
> > > > >>>>
> > > > >>>> If we change to the default value of 0: No limit, this example is
> > > > >>>> treated as a single block and all ‘=’ are aligned.
> > > > >>>>
> > > > >>>> Is there a reason ‘4’ was selected?Is there is any harm in using the
> > > > >>>> default of 0?
> > > > >>>>
> > > > >>> We can certainly change the threshold. '4' was derived by
> > > > >>> experimentation. This is an area that is somewhat subjective and every
> > > > >>> case is difficult to cover well. Please feel free to suggest any changes.
> > > > >>>
> > > > >>
> > > > >> I recommend we use the default value of 0.  That way, any code that
> choose to
> > > > >> align assignments will still be aligned.  If a developer does not like
> > > > >> short assignments and long assignments in the same code block to use
> the
> > > > >> long assignment column, they can always break the block up into
> multiple
> > > > >> blocks by adding a carriage return.
> > > > >>
> > > > >>> For the benefit of others, this file can be a useful refernce
> > > > >>> understanding spans, gaps, and thresholds.
> > > > >>>
> > > > >>>
> https://github.com/uncrustify/uncrustify/blob/master/documentation/htdocs/c
> onfiguration.txt
> > > > >>>
> > > > >>>>    * Example #2: Uncruistfy is confused by the DEBUG_CODE()
> macro.This is
> > > > >>>>      not a traditional macro because the
> > > > >>>>
> > > > >>>> contents of the macro is a block of C code.I do not know how to
> convince
> > > > >>>> uncrustify that the contents of a
> > > > >>>>
> > > > >>>> macro like function call to be treated as a code block from an indent
> > > > >>>> perspective.
> > > > >>>>
> > > > >>> I believe this would have to be overridden in the Uncrustify fork since
> > > > >>> DEBUG_CODE() is being treated as a macro function call and code
> blocks
> > > > >>> are not expected there. In addition, some special treatment might be
> > > > >>> needed for alignment in between
> DEBUG_CODE_BEGIN()/DEBUG_CODE_END().
> > > > >>>
> > > > >>> I'm happy to look at this. However, regression validation can take a
> > > > >>> while so I'd like to make sure we need this now. In cases that do not
> > > > >>> have additional code blocks, it seems to format fairly well. Is this
> > > > >>> prevalent and impactful enough it must be fixed now? Or, could we
> > > > >>> revisit it with a follow up patch?
> > > > >>
> > > > >> How long does regression testing take?  I see about 20 instances of
> > > > >> DEBUG_CODE() that are producing bad formatting.  If the content inside
> > > > >> DEBUG_CODE() is a single line of a single block of statements without
> > > > >> any if/while/for/case statements that require further indent, then the
> > > > >> format looks ok.
> > > > >>
> > > > >> DEBUG_CODE_BEGIN() and DEBUG_CODE_END() would look better if
> the
> > > > >> contents between are also indented one level.
> > > > >>
> > > > >>>
> > > > >>>>    * Example #3/#4: Uncrustify is confused by the OPTIONAL
> keyword.The
> > > > >>>>      edk2 config declares it as a QUALIFIER
> > > > >>>>
> > > > >>>> like IN and OUT.However, OPTIONAL only appears at the end of the
> line
> > > > >>>> that declares a parameter to a
> > > > >>>>
> > > > >>>> function.There are 3 forms. One with comma after OPTIONAL.One
> with comma
> > > > >>>> before OPTIONAL, and
> > > > >>>>
> > > > >>>> one with no comma if the parameter is the last parameter in the
> function
> > > > >>>> declaration.
> > > > >>>>
> > > > >>>> TYPE ParamName OPTIONAL,
> > > > >>>>
> > > > >>>> TYPEParamName, OPTIONAL
> > > > >>>>
> > > > >>>> TYPEParamName OPTIONAL
> > > > >>>>
> > > > >>>> OPTIONAL is defined to nothing in edk2 builds.From a uncrustify
> > > > >>>> perspective, we really want is to be
> > > > >>>>
> > > > >>>> ignored or more correctly treated it as a token that is attached to
> > > > >>>> ParamName and the combination of
> > > > >>>>
> > > > >>>> ParamName and OPTIONAL treated as one unit to determine indents.
> > > > >>>>
> > > > >>> "TYPE ParamName, OPTIONAL" seems especially problematic and
> presents an
> > > > >>> inconsistency with the other formats. Mike, can you please let me
> know
> > > > >>> if you have the same observation and your thoughts on a consistent
> pattern?
> > > > >>>
> > > > >>>> Thanks,
> > > > >>>>
> > > > >>>> Mike
> > > > >>>>
> > > > >>>> *From:*devel@edk2.groups.io <devel@edk2.groups.io> *On Behalf
> Of *Maciej
> > > > >>>> Rabeda
> > > > >>>> *Sent:* Thursday, December 2, 2021 10:27 AM
> > > > >>>> *To:* devel@edk2.groups.io; Kinney, Michael D
> > > > >>>> <michael.d.kinney@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
> > > > >>>>
> > > > >>>> Hey Mike,
> > > > >>>>
> > > > >>>> While most of the changes related to fixing coding style violations,
> > > > >>>> there are a couple of changes to NetworkPkg in that PR that make
> the
> > > > >>>> code less readable. Examples below.
> > > > >>>>
> > > > >>>> Example 1:
> > > > >>>>
> > > > >>>>
> > > > >>>>
> > > > >>>> Example 2:
> > > > >>>>
> > > > >>>>
> > > > >>>> Example 3:
> > > > >>>>
> > > > >>>>
> > > > >>>> Example 4:
> > > > >>>>
> > > > >>>> On 30-Nov-21 23:34, Michael D Kinney wrote:
> > > > >>>>
> > > > >>>>      Hello,
> > > > >>>>
> > > > >>>>      Thank you for your patience during this extended hard freeze.
> > > > >>>>
> > > > >>>>      Just one more step to go.There has been a delay in the review of
> > > > >>>>
> > > > >>>>      the patch series with the uncrustify source changes.PR(6).This
> > > > >>>>
> > > > >>>>      patch series was not sent out as patch review email because of its
> > > > >>>>
> > > > >>>>      very large size.It only contains source style changes and the
> > > > >>>>
> > > > >>>>      CompareBuild tool and GitHub action has shown there are no
> binary
> > > > >>>>
> > > > >>>>      differences introduced with these source style changes.
> > > > >>>>
> > > > >>>>      If you are a package maintainer, then please review the following
> > > > >>>>
> > > > >>>>      branch/PR for your package contents and review the EDK II CI
> results
> > > > >>>>
> > > > >>>>      and BuildCompare results.I do not expect a line by line review
> > > > >>>>
> > > > >>>>      because we already had time to provide feedback on the source
> style
> > > > >>>>
> > > > >>>>      performed by uncrustify.Instead, a Reviewed-by for your package
> > > > >>>>
> > > > >>>>      indicates that you have reviewed the EDK II CI results and
> CompareBuild
> > > > >>>>
> > > > >>>>      tool functionality and results and you accept the source style
> > > > >>>>
> > > > >>>>      changes to your package.
> > > > >>>>
> > > > >>>>
> *https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrustifyCha
> nges_V5
> > > > >>>
> <https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrustifyCha
> nges_V5>
> > > > >>>>
> > > > >>>>      *https://github.com/tianocore/edk2/pull/2229
> <https://github.com/tianocore/edk2/pull/2229>
> > > > >>>>
> > > > >>>>      *https://github.com/mdkinney/edk2/actions/runs/1521618836
> > > > >>> <https://github.com/mdkinney/edk2/actions/runs/1521618836>
> > > > >>>>
> > > > >>>>      Additional details on this update below.
> > > > >>>>
> > > > >>>>      Thank you,
> > > > >>>>
> > > > >>>>      Mike
> > > > >>>>
> > > > >>>>      Changes from Update #3
> > > > >>>>
> > > > >>>>      ----------------------------------------------------------------------------
> > > > >>>>
> > > > >>>>      * Pushed PR (5)
> > > > >>>>
> > > > >>>>      * Added link to PR(6). EDK II CI Status is PASS. Build Compare PASS.
> > > > >>>>
> > > > >>>>      * Waiting for review of PR (6)
> > > > >>>>
> > > > >>>>      * Review of PR (7) completed and waiting for review of PR (6)
> > > > >>>>
> > > > >>>>      ----------------------------------------------------------------------------
> > > > >>>>
> > > > >>>>      Changes from Update #2
> > > > >>>>
> > > > >>>>      ----------------------------------------------------------------------------
> > > > >>>>
> > > > >>>>      * Changed order of PRs swapping (4) and (5).The PR that activates
> > > > >>>>
> > > > >>>>      increases the max CI agent job time is independent of all the other
> > > > >>>>
> > > > >>>>      PRs and its review is complete, so it can be committed now.
> > > > >>>>
> > > > >>>>      * Pushed PRs (1), (2), (3), (4).
> > > > >>>>
> > > > >>>>      * Waiting for review to complete for PRs (5) and (6)
> > > > >>>>
> > > > >>>>      * Reviews complete for PR (7)
> > > > >>>>
> > > > >>>>      * Identifies steps using git filter-branch to apply uncrustify changes
> to a
> > > > >>>>
> > > > >>>>      code review patch series that was generated before the uncrustify
> changes
> > > > >>>>
> > > > >>>>      avoiding manual merge.
> > > > >>>>
> > > > >>>>      * Identified steps using git filter-repo to generate an alternate
> history of
> > > > >>>>
> > > > >>>>      the edk2 repo with uncrustify changes applied on every
> commit.This may
> > > > >>>>
> > > > >>>>      be useful when evaluating changes to files using tools like git
> blame
> > > > >>>>
> > > > >>>>      without the large uncrustify patch series.
> > > > >>>>
> > > > >>>>      ---------------------------------------------------------------------------
> > > > >>>>
> > > > >>>>      Changes from Update #1
> > > > >>>>
> > > > >>>>      ----------------------------------------------------------------------------
> > > > >>>>
> > > > >>>>      * Changed order of PRs swapping (6) and (7).The PR that activates
> > > > >>>>
> > > > >>>>      EDK II CI check UncrustifyCheck has to be last because it
> unconditionally
> > > > >>>>
> > > > >>>>      checks all C/H files in all packages.Not just files that have been
> > > > >>>>
> > > > >>>>      modified like some of the other checkers.
> > > > >>>>
> > > > >>>>      * Updated link to the branch with the UncrustifyCheck plugin that
> has been
> > > > >>>>
> > > > >>>>      updated with a one line change and Reviewed-by and Tested-by
> tags.
> > > > >>>>
> > > > >>>>
> https://github.com/mdkinney/edk2/tree/Bug_3748_add_uncrustify_ci_plugin_v
> 6
> > > > >>>
> <https://github.com/mdkinney/edk2/tree/Bug_3748_add_uncrustify_ci_plugin_
> v6>
> > > > >>>>
> > > > >>>>      * Reviews complete for (1), (2), (3), (5), and (7)
> > > > >>>>
> > > > >>>>      ---------------------------------------------------------------------------
> > > > >>>>
> > > > >>>>      Michael Kubacki and I have prepared the patches required to apply
> the
> > > > >>>>
> > > > >>>>      uncrustify changes and enable EDK II CI to check all submitted
> > > > >>>>
> > > > >>>>      patches have been run through uncrustify.
> > > > >>>>
> > > > >>>>      We have verified through the CompareBuild GitHub Action that the
> > > > >>>>
> > > > >>>>      format changes performed by uncrustify have no functional
> changes.
> > > > >>>>
> > > > >>>>      All of the OBJ, LIB, DLL, EFI, FFS, FV, and FD files match 100%
> > > > >>>>
> > > > >>>>      across 70 VS2019/GCC5 builds of all package/platform DSC files in
> > > > >>>>
> > > > >>>>      the edk2 repo.
> > > > >>>>
> > > > >>>>      The hard freeze will be extended after the edk2-stable202111 tag
> until
> > > > >>>>
> > > > >>>>      all uncrustify related changes are committed.We do not expect
> this
> > > > >>>>
> > > > >>>>      to take more than a few days.Do not push any PRs until the hard
> > > > >>>>
> > > > >>>>      freeze is lifted.
> > > > >>>>
> > > > >>>>      The changes are broken up into 7 patch series/PRs.The PRs are
> ordered
> > > > >>>>
> > > > >>>>      so they can be submitted using the normal submission process and
> EDK II
> > > > >>>>
> > > > >>>>      CI will pass for each one.Details are listed below.
> > > > >>>>
> > > > >>>>      Uncrustify 73.0.3 for EDK II
> > > > >>>>
> > > > >>>>      =============================
> > > > >>>>
> > > > >>>>      * Sources:https://dev.azure.com/projectmu/_git/Uncrustify
> <https://dev.azure.com/projectmu/_git/Uncrustify>
> > > > >>>>
> > > > >>>>      *
> Documentation:https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncru
> stify.wiki/1/Project-Mu-(EDK-
> > II)-
> > > > Fork-
> > > > >>> Readme
> <https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Pr
> oject-Mu-(EDK-II)-Fork-Readme>
> > > > >>>>
> > > > >>>>      *
> Download:https://dev.azure.com/projectmu/Uncrustify/_packaging?_a=packag
> e&feed=mu_uncrustify&package=mu-
> > > > >>> uncrustify-release&protocolType=NuGet&version=73.0.3
> > > > >>>
> <https://dev.azure.com/projectmu/Uncrustify/_packaging?_a=package&feed=
> mu_uncrustify&package=mu-uncrustify-
> > > > >>> release&protocolType=NuGet&version=73.0.3>
> > > > >>>>
> > > > >>>>      Installing Uncrustify
> > > > >>>>
> > > > >>>>      ======================
> > > > >>>>
> > > > >>>>      The Uncrustify tool is installed automatically when the Pytools
> > > > >>>>
> > > > >>>>      environment is used and the stuart* commands are run to
> complete the
> > > > >>>>
> > > > >>>>      environment setup.Please see:
> > > > >>>>
> > > > >>>>      https://github.com/tianocore/edk2/tree/master/.pytool#running-
> ci-locally
> > > > >>> <https://github.com/tianocore/edk2/tree/master/.pytool#running-ci-
> locally>
> > > > >>>>
> > > > >>>>      Uncrustify can also be installed from the download page listed
> above
> > > > >>>>
> > > > >>>>      or built from sources from the source link above.
> > > > >>>>
> > > > >>>>      The Documentation link provides instruction on how to run
> uncrustify from
> > > > >>>>
> > > > >>>>      the command line or install as a Visual Studio Code plugin.The
> main
> > > > >>>>
> > > > >>>>      uncrustify documentation also describes how to integrate with a
> few other
> > > > >>>>
> > > > >>>>      editors.
> > > > >>>>
> > > > >>>>      We have also discussed a client side githook.That effort has not
> started.
> > > > >>>>
> > > > >>>>      Let us know if that is a feature you would find useful.
> > > > >>>>
> > > > >>>>      Developer impact for new code reviews
> > > > >>>>
> > > > >>>>      ======================================
> > > > >>>>
> > > > >>>>      Once the uncrustify checker is active in EDK II CI, developers must
> > > > >>>>
> > > > >>>>      make sure their patches are run through the uncrustify tool before
> > > > >>>>
> > > > >>>>      sending the patches for review.
> > > > >>>>
> > > > >>>>      Developers must install and run uncrustify against changes files
> before
> > > > >>>>
> > > > >>>>      sending patch review emails or submitting PR for EDK II CI.If EDK II
> CI
> > > > >>>>
> > > > >>>>      detects and differences in source formatting, then EDK II CI will fail
> > > > >>>>
> > > > >>>>      and the developer must run uncrustify and resubmit the patches.
> > > > >>>>
> > > > >>>>      Developer impact to patch series/PRs reviewed during edk2-
> stable201121 soft/hard freeze
> > > > >>>>
> > > > >>>>
> =================================================================
> ======================
> > > > >>>>
> > > > >>>>      Developers must rebase their changes after the uncrustify source
> changes are
> > > > >>>>
> > > > >>>>      committed.The branch with a preview of the uncrustify changes
> can be used
> > > > >>>>
> > > > >>>>      to start this rebase work.
> > > > >>>>
> > > > >>>>
> https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrustifyChan
> ges_V5
> > > > >>>
> <https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrustifyCha
> nges_V5>
> > > > >>>>
> > > > >>>>      The following steps can be used to update an existing branch with
> the
> > > > >>>>
> > > > >>>>      required uncrustify format.This is the Windows version.I will add
> > > > >>>>
> > > > >>>>      the Linux version soon.
> > > > >>>>
> > > > >>>>      1) Fetch and checkout and rebase to latest edk2/master
> > > > >>>>
> > > > >>>>      git fetch origin
> > > > >>>>
> > > > >>>>      git checkout master
> > > > >>>>
> > > > >>>>      git rebase origin/master
> > > > >>>>
> > > > >>>>      2) Make a backup copy of plugin UncrustifyCheck outside
> WORKSPACE.
> > > > >>>>
> > > > >>>>      (e.g. C:\Temp\UncrustifyCheck) so the uncrustify tool executable
> and
> > > > >>>>
> > > > >>>>      EDK II specific uncrustify configuration file available when working
> > > > >>>>
> > > > >>>>      with a branch that does not have those tools in its scope.
> > > > >>>>
> > > > >>>>      xcopy .pytool\Plugin\UncrustifyCheck C:\Temp\UncrustifyCheck
> > > > >>>>
> > > > >>>>      3) Check out the patch series branch (e.g. MyBranch)
> > > > >>>>
> > > > >>>>      git checkout MyBranch
> > > > >>>>
> > > > >>>>      4) Rebase patch series against edk2-stable202111
> > > > >>>>
> > > > >>>>
> > > > >>>>
> > > > >>>>      git rebase edk2-stable202111
> > > > >>>>
> > > > >>>>      5) Create new branch for the uncrustifed version (e.g.
> MyBranch_Uncrustified)
> > > > >>>>
> > > > >>>>      git checkout -b MyBranch_Uncrustified
> > > > >>>>
> > > > >>>>      6) Use git filter-branch to uncrustify all the commits in the series
> > > > >>>>
> > > > >>>>      between the rebase target from (2) and HEAD of the branch.A filter
> > > > >>>>
> > > > >>>>      can be used to scope the uncrustify operations to only the C/H files
> > > > >>>>
> > > > >>>>      in the specific package the patch series is against. (e.g.
> DynamicTablesPkg).
> > > > >>>>
> > > > >>>>      BaseTools should always be excluded.If the package scoped filter
> is
> > > > >>>>
> > > > >>>>      not used, it will still work, but will take longer to run because
> > > > >>>>
> > > > >>>>      uncrustify will rescan every C/H files in the whole repo.
> > > > >>>>
> > > > >>>>      git filter-branch --tree-filter "git ls-files DynamicTablesPkg*.c
> DynamicTablesPkg*.h :!BaseTools/* |
> > > > >>> c:\\Temp\\UncrustifyCheck\\mu-uncrustify-
> release_extdep\\Windows-x86\\uncrustify.exe -c
> > > > >>> c:\\Temp\\UncrustifyCheck\\uncrustify.cfg -F - --replace --no-backup -
> -if-changed" edk2-stable202111..HEAD
> > > > >>>>
> > > > >>>>      7) Now that all the individual patches in the branch are uncrustified,
> > > > >>>>
> > > > >>>>      rebase against latest edk2/master that is already uncrustified.
> > > > >>>>
> > > > >>>>      git rebase master
> > > > >>>>
> > > > >>>>      8) Verify the patches in this new branch.
> > > > >>>>
> > > > >>>>      Impacts to tracing history across the uncrusity changes
> > > > >>>>
> > > > >>>>
> =======================================================
> > > > >>>>
> > > > >>>>      Tools the view file and line history do work with the large
> uncrustify
> > > > >>>>
> > > > >>>>      patch series.One impact is that the operations can be very slow
> due
> > > > >>>>
> > > > >>>>      to the large uncrustify patches.
> > > > >>>>
> > > > >>>>      One option to provide a faster experience is to provide an
> alternate
> > > > >>>>
> > > > >>>>      version of the edk2 repository as "documentation" that has the
> > > > >>>>
> > > > >>>>      entire history re-written with uncrustify run on every commit.
> > > > >>>>
> > > > >>>>      The tool called git-filter-repo can be used to perform this
> > > > >>>>
> > > > >>>>      transformation and runs in a reasonable period of time (a few
> hours)
> > > > >>>>
> > > > >>>>      https://github.com/newren/git-filter-repo
> <https://github.com/newren/git-filter-repo>
> > > > >>>>
> > > > >>>>      https://github.com/newren/git-filter-
> repo/blob/main/contrib/filter-repo-demos/lint-history
> > > > >>> <https://github.com/newren/git-filter-repo/blob/main/contrib/filter-
> repo-demos/lint-history>
> > > > >>>>
> > > > >>>>      The following steps can be used to perform this transformation.
> > > > >>>>
> > > > >>>>      This is the Windows version. I will add the Linux version soon.
> > > > >>>>
> > > > >>>>      ** WARNING **This operation modifies(rewrites) all the commits
> > > > >>>>
> > > > >>>>      in the local copy of the repo.Do not perform
> > > > >>>>
> > > > >>>>      these steps on a local repo you are using for
> > > > >>>>
> > > > >>>>      active development.
> > > > >>>>
> > > > >>>>      1) Clone edk2 into a new directory (see **WARNING**)
> > > > >>>>
> > > > >>>>      git clonehttps://github.com/tianocore/edk2.git
> <https://github.com/tianocore/edk2.git>  edk2-uncrustified
> > > > >>>>
> > > > >>>>      cd edk2-uncrustified
> > > > >>>>
> > > > >>>>      2) Setup python virtual env, install pytools, and run stuart
> commands
> > > > >>>>
> > > > >>>>      to setup build environment which includes installing uncrustify
> tools.
> > > > >>>>
> > > > >>>>      https://github.com/tianocore/edk2/tree/master/.pytool#running-
> ci-locally
> > > > >>> <https://github.com/tianocore/edk2/tree/master/.pytool#running-ci-
> locally>
> > > > >>>>
> > > > >>>>      3) Make a backup copy of plugin UncrustifyCheck outside
> WORKSPACE.
> > > > >>>>
> > > > >>>>      (e.g. C:\Temp\UncrustifyCheck) so the uncrustify tool executable
> and
> > > > >>>>
> > > > >>>>      EDK II specific uncrustify configuration file available when working
> > > > >>>>
> > > > >>>>      with a branch that does not have those tools in its scope.
> > > > >>>>
> > > > >>>>      xcopy .pytool\Plugin\UncrustifyCheck C:\Temp\UncrustifyCheck
> > > > >>>>
> > > > >>>>      4) Use lint-history.py from git-filter-repo examples
> > > > >>>>
> > > > >>>>      https://github.com/newren/git-filter-repo
> <https://github.com/newren/git-filter-repo>
> > > > >>>>
> > > > >>>>      https://github.com/newren/git-filter-
> repo/blob/main/contrib/filter-repo-demos/lint-history
> > > > >>> <https://github.com/newren/git-filter-repo/blob/main/contrib/filter-
> repo-demos/lint-history>
> > > > >>>>
> > > > >>>>      Line #127 - Add try except around subprocess.check_call() with
> except
> > > > >>>>
> > > > >>>>      being pass.This is required because there are a few commits of C
> > > > >>>>
> > > > >>>>      files in the edk2 repo that have incorrect C syntax and do not
> > > > >>>>
> > > > >>>>      build with a C compiler and break the uncrustify parser.Skip
> reformat
> > > > >>>>
> > > > >>>>      of C files that can not be parsed by uncrustify.These rare instances
> > > > >>>>
> > > > >>>>      are addressed in the commit that fixes the C syntax error.
> > > > >>>>
> > > > >>>>      Run this slightly modified version of lint-history.Include only
> > > > >>>>
> > > > >>>>      C/H files and exclude directories that start with 'Tools' or
> 'BaseTools'.
> > > > >>>>
> > > > >>>>      This step took about 2.2 hours on a laptop.
> > > > >>>>
> > > > >>>>      lint-history.py
> > > > >>>>
> > > > >>>>      --relevant "return (not filename.startswith(b'Tools') and not
> filename.startswith(b'BaseTools') and
> > > > >>> (filename.endswith(b'.c') or filename.endswith(b'.h')))"
> > > > >>>>
> > > > >>>>      c:\\work\\GitHub\\tianocore\\foo\\UncrustifyCheck\\mu-
> uncrustify-release_extdep\\Windows-x86\\uncrustify.exe
> > -
> > > c
> > > > >>> c:\\work\\GitHub\\tianocore\\foo\\UncrustifyCheck\\uncrustify.cfg --
> replace --no-backup --if-changed
> > > > >>>>
> > > > >>>>      Order of PRs to apply during extended hard freeze
> > > > >>>>
> > > > >>>>      ==================================================
> > > > >>>>
> > > > >>>>      1) Update EmulatorPkg Win Host [BuildOptions] MSFT CC_FLAGS
> to not force debug information
> > > > >>>>
> > > > >>>>      *https://bugzilla.tianocore.org/show_bug.cgi?id=3747
> <https://bugzilla.tianocore.org/show_bug.cgi?id=3747>
> > > > >>>>
> > > > >>>>
> *https://github.com/mdkinney/edk2/tree/Bug_3747_EmulatorPkg_WinHost_Re
> producibleBuild
> > > > >>>
> <https://github.com/mdkinney/edk2/tree/Bug_3747_EmulatorPkg_WinHost_Re
> producibleBuild>
> > > > >>>>
> > > > >>>>      *https://github.com/tianocore/edk2/pull/2215
> <https://github.com/tianocore/edk2/pull/2215>
> > > > >>>>
> > > > >>>>      * Required for EmulatorPkg to pass CompareBuild for VS2019
> IA32/X64 builds.
> > > > >>>>
> > > > >>>>      * Status: Review complete.PR pushed.
> > > > >>>>
> > > > >>>>      2) EccCheck should not revert staged and local changes
> > > > >>>>
> > > > >>>>      *https://bugzilla.tianocore.org/show_bug.cgi?id=2986
> <https://bugzilla.tianocore.org/show_bug.cgi?id=2986>
> > > > >>>>
> > > > >>>>
> *https://github.com/mdkinney/edk2/tree/Bug_2986_EccCheckRemoveGitRever
> t_V2
> > > > >>>
> <https://github.com/mdkinney/edk2/tree/Bug_2986_EccCheckRemoveGitRever
> t_V2>
> > > > >>>>
> > > > >>>>      *https://github.com/tianocore/edk2/pull/2216
> <https://github.com/tianocore/edk2/pull/2216>
> > > > >>>>
> > > > >>>>      * Required for EDK II CI to complete in a reasonable period of time
> when
> > > > >>>>
> > > > >>>>      processing the 4000+ source file style changes made by uncrustify.
> > > > >>>>
> > > > >>>>      * Also fixes critical bugs that can potentially corrupt git state when
> > > > >>>>
> > > > >>>>      EccCheck is run locally.
> > > > >>>>
> > > > >>>>      * Status: Review complete.PR pushed.
> > > > >>>>
> > > > >>>>      3) Update pytool LicenseCheck plugin to use temp directory for diff
> output file
> > > > >>>>
> > > > >>>>      *https://bugzilla.tianocore.org/show_bug.cgi?id=3746
> <https://bugzilla.tianocore.org/show_bug.cgi?id=3746>
> > > > >>>>
> > > > >>>>
> *https://github.com/mdkinney/edk2/tree/Bug_3746_LicenseCheckUseDiffOutp
> utFile_V2
> > > > >>>
> <https://github.com/mdkinney/edk2/tree/Bug_3746_LicenseCheckUseDiffOutp
> utFile_V2>
> > > > >>>>
> > > > >>>>      *https://github.com/tianocore/edk2/pull/2217
> <https://github.com/tianocore/edk2/pull/2217>
> > > > >>>>
> > > > >>>>      * Required to reduce EDK II CI build times.
> > > > >>>>
> > > > >>>>      * Status: Review complete.PR pushed.
> > > > >>>>
> > > > >>>>      4) Update max job time from 60 min to 120 minutes
> in .azurepipelines/templates
> > > > >>>>
> > > > >>>>      *https://bugzilla.tianocore.org/show_bug.cgi?id=3750
> <https://bugzilla.tianocore.org/show_bug.cgi?id=3750>
> > > > >>>>
> > > > >>>>
> *https://github.com/mdkinney/edk2/tree/Bug_3750_IncreaseAzurePipelinesTim
> eout
> > > > >>>
> <https://github.com/mdkinney/edk2/tree/Bug_3750_IncreaseAzurePipelinesTim
> eout>
> > > > >>>>
> > > > >>>>      *https://github.com/tianocore/edk2/pull/2219
> <https://github.com/tianocore/edk2/pull/2219>
> > > > >>>>
> > > > >>>>      * Required to allow EccCheck of uncrustify changes to complete
> on Azure
> > > > >>>>
> > > > >>>>      Pipelines CI agents without timing out.
> > > > >>>>
> > > > >>>>      * Status: Review complete.PR pushed.
> > > > >>>>
> > > > >>>>      5) Update Package YAML to ignore specific ECC files/errors
> > > > >>>>
> > > > >>>>      *https://bugzilla.tianocore.org/show_bug.cgi?id=3749
> <https://bugzilla.tianocore.org/show_bug.cgi?id=3749>
> > > > >>>>
> > > > >>>>
> *https://github.com/mdkinney/edk2/tree/Bug_3749_EccCheckIgnoreFilesErrors
> > > > >>>
> <https://github.com/mdkinney/edk2/tree/Bug_3749_EccCheckIgnoreFilesErrors>
> > > > >>>>
> > > > >>>>      *https://github.com/tianocore/edk2/pull/2218
> <https://github.com/tianocore/edk2/pull/2218>
> > > > >>>>
> > > > >>>>      * Required to pass EccCheck
> > > > >>>>
> > > > >>>>      * Status: Review complete. PR pushed
> > > > >>>>
> > > > >>>>      6) Uncrustify Source Changes
> > > > >>>>
> > > > >>>>      *https://bugzilla.tianocore.org/show_bug.cgi?id=3737
> <https://bugzilla.tianocore.org/show_bug.cgi?id=3737>
> > > > >>>>
> > > > >>>>      *https://bugzilla.tianocore.org/show_bug.cgi?id=3739
> <https://bugzilla.tianocore.org/show_bug.cgi?id=3739>
> > > > >>>>
> > > > >>>>
> *https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrustifyCha
> nges_V5
> > > > >>>
> <https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrustifyCha
> nges_V5>
> > > > >>>>
> > > > >>>>      *https://github.com/tianocore/edk2/pull/2229
> <https://github.com/tianocore/edk2/pull/2229>
> > > > >>>>
> > > > >>>>      * Build comparison result
> PASS:https://github.com/mdkinney/edk2/actions/runs/1521618836
> > > > >>> <https://github.com/mdkinney/edk2/actions/runs/1521618836>
> > > > >>>>
> > > > >>>>      * EFI_D_ -> DEBUG changes required to pass PatchCheck
> > > > >>>>
> > > > >>>>      * Uncrustify format changes required to pass UncrustifyCheck
> > > > >>>>
> > > > >>>>      * Status:
> > > > >>>>
> > > > >>>>      Waiting for review
> > > > >>>>
> > > > >>>>      7) UncrustifyCheck EDK II CI Plugin
> > > > >>>>
> > > > >>>>      *https://bugzilla.tianocore.org/show_bug.cgi?id=3748
> <https://bugzilla.tianocore.org/show_bug.cgi?id=3748>
> > > > >>>>
> > > > >>>>
> *https://github.com/mdkinney/edk2/tree/Bug_3748_add_uncrustify_ci_plugin_
> v6
> > > > >>>
> <https://github.com/mdkinney/edk2/tree/Bug_3748_add_uncrustify_ci_plugin_
> v6>
> > > > >>>>
> > > > >>>>      * Required to enforce all PRs submitted to EDK II CI match
> uncrustify format.
> > > > >>>>
> > > > >>>>      * Unconditionally checks all packages.Can not be committed until
> all C/H
> > > > >>>>
> > > > >>>>      source files have been updated.
> > > > >>>>
> > > > >>>>      * Status: Review complete
> > > > >>>>
> > > > >>>>      Combined Branch/PR for Review/Test
> > > > >>>>
> > > > >>>>      ==================================
> > > > >>>>
> > > > >>>>      * Build Comparison results must pass 100% across the full set of
> PRs before
> > > > >>>>
> > > > >>>>      the individual PRs can be pushed in the order listed above.
> > > > >>>>
> > > > >>>>      *
> Branch:https://github.com/mdkinney/edk2/tree/TestOnly_Uncrustify_PR_Series
> > > > >>>
> <https://github.com/mdkinney/edk2/tree/TestOnly_Uncrustify_PR_Series>
> > > > >>>>
> > > > >>>>      * PR:https://github.com/tianocore/edk2/pull/2229
> <https://github.com/tianocore/edk2/pull/2229>
> > > > >>>>
> > > > >>>>      Status = PASS
> > > > >>>>
> > > > >>>>      * CompareBuild:
> > > > >>>>
> > > > >>>>
> Branch:https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrusti
> fyChanges_V5
> > > > >>>
> <https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrustifyCha
> nges_V5>
> > > > >>>>
> > > > >>>>      --ref1:ef9a059cdb15844fe52a49af2bf7d86b9dd3e9bf
> > > > >>>>
> > > > >>>>      --ref2:Bug_3737_3739_ApplyUncrustifyChanges_V5
> > > > >>>>
> > > > >>>>      Extra Options: -n 4 --quiet
> > > > >>>>
> > > > >>>>
> Results:https://github.com/mdkinney/edk2/actions/runs/1521618836
> > > > >>> <https://github.com/mdkinney/edk2/actions/runs/1521618836>
> > > > >>>>
> > > > >>>>      30 VS2019 build comparisons PASS
> > > > >>>>
> > > > >>>>      40 GCC5 build comparisons PASS
> > > > >>>>
> > > > >>>>      100% PASS
> > > > >>>>
> > > > >>>>      The following git log shows the set of patches from --ref1 to --ref
> 2across
> > > > >>>>
> > > > >>>>      which there are no differences in any of the
> OBJ/LIB/DLL/EFI/FFS/FV/FD files.
> > > > >>>>
> > > > >>>>      --ref2
> > > > >>>>
> > > > >>>>      b7d4bf0675b7 (HEAD ->
> Bug_3737_3739_ApplyUncrustifyChanges_V5) UnitTestFrameworkPkg: Apply
> uncrusitify
> > changes
> > > > >>>>
> > > > >>>>      7f03d25f60e7 UefiPayloadPkg: Apply uncrusitify changes
> > > > >>>>
> > > > >>>>      0bfd8d9b5ac9 UefiCpuPkg: Apply uncrusitify changes
> > > > >>>>
> > > > >>>>      e1cd9bfb9dea StandaloneMmPkg: Apply uncrusitify changes
> > > > >>>>
> > > > >>>>      5da2f65be378 SourceLevelDebugPkg: Apply uncrusitify changes
> > > > >>>>
> > > > >>>>      95b86de07e5d SignedCapsulePkg: Apply uncrusitify changes
> > > > >>>>
> > > > >>>>      fe71d97246c4 ShellPkg: Apply uncrusitify changes
> > > > >>>>
> > > > >>>>      54c21c952992 SecurityPkg: Apply uncrusitify changes
> > > > >>>>
> > > > >>>>      187a3785f12b RedfishPkg: Apply uncrusitify changes
> > > > >>>>
> > > > >>>>      810100002a46 PcAtChipsetPkg: Apply uncrusitify changes
> > > > >>>>
> > > > >>>>      276a695c0cf2 OvmfPkg: Apply uncrusitify changes
> > > > >>>>
> > > > >>>>      303c0a91ab07 NetworkPkg: Apply uncrusitify changes
> > > > >>>>
> > > > >>>>      bc80792cd1b1 MdePkg: Apply uncrusitify changes
> > > > >>>>
> > > > >>>>      3ea86be17a2a MdeModulePkg: Apply uncrusitify changes
> > > > >>>>
> > > > >>>>      c70ef11ed0cd IntelFsp2WrapperPkg: Apply uncrusitify changes
> > > > >>>>
> > > > >>>>      c0291221f252 IntelFsp2Pkg: Apply uncrusitify changes
> > > > >>>>
> > > > >>>>      6a479952a690 FmpDevicePkg: Apply uncrusitify changes
> > > > >>>>
> > > > >>>>      3a7c05b7070d FatPkg: Apply uncrusitify changes
> > > > >>>>
> > > > >>>>      b789f98c8959 EmulatorPkg: Apply uncrusitify changes
> > > > >>>>
> > > > >>>>      952d7a1c9220 EmbeddedPkg: Apply uncrusitify changes
> > > > >>>>
> > > > >>>>      a1cc9881bab6 DynamicTablesPkg: Apply uncrusitify changes
> > > > >>>>
> > > > >>>>      50654dfe5785 CryptoPkg: Apply uncrusitify changes
> > > > >>>>
> > > > >>>>      ed965a02dfa1 ArmVirtPkg: Apply uncrusitify changes
> > > > >>>>
> > > > >>>>      9744023fbc46 ArmPlatformPkg: Apply uncrusitify changes
> > > > >>>>
> > > > >>>>      7a1cde5f5bba ArmPkg: Apply uncrusitify changes
> > > > >>>>
> > > > >>>>      19d17e0913e8 UefiCpuPkg: Change use of EFI_D_* to DEBUG_*
> > > > >>>>
> > > > >>>>      ffa718b4f994 SourceLevelDebugPkg: Change use of EFI_D_* to
> DEBUG_*
> > > > >>>>
> > > > >>>>      b86cb3c5e5b4 ShellPkg: Change use of EFI_D_* to DEBUG_*
> > > > >>>>
> > > > >>>>      c7c42204dc07 SecurityPkg: Change use of EFI_D_* to DEBUG_*
> > > > >>>>
> > > > >>>>      16b8e6f958e4 PcAtChipsetPkg: Change use of EFI_D_* to
> DEBUG_*
> > > > >>>>
> > > > >>>>      0ac3f8b2dac5 OvmfPkg: Change use of EFI_D_* to DEBUG_*
> > > > >>>>
> > > > >>>>      bc5004b8d294 NetworkPkg: Change use of EFI_D_* to DEBUG_*
> > > > >>>>
> > > > >>>>      6f671a8e2377 MdePkg: Change use of EFI_D_* to DEBUG_*
> > > > >>>>
> > > > >>>>      a10c610ff9a3 MdeModulePkg: Change use of EFI_D_* to
> DEBUG_*
> > > > >>>>
> > > > >>>>      09a3bddba390 FatPkg: Change use of EFI_D_* to DEBUG_*
> > > > >>>>
> > > > >>>>      59c61318246a EmulatorPkg: Change use of EFI_D_* to DEBUG_*
> > > > >>>>
> > > > >>>>      3a80367dda3b EmbeddedPkg: Change use of EFI_D_* to DEBUG_*
> > > > >>>>
> > > > >>>>      23eb1aaf80ca ArmVirtPkg: Change use of EFI_D_* to DEBUG_*
> > > > >>>>
> > > > >>>>      875914b45c54 ArmPlatformPkg: Change use of EFI_D_* to
> DEBUG_*
> > > > >>>>
> > > > >>>>      eb2eca82b451 ArmPkg: Change use of EFI_D_* to DEBUG_*
> > > > >>>>
> > > > >>>>      f0f3f5aae7c4 (origin/master, origin/HEAD, master)
> UnitTestFrameworkPkg: Update YAML to ignore specific ECC
> > > > >>> files/errors
> > > > >>>>
> > > > >>>>      c05734797790 UefiPayloadPkg: Update YAML to ignore specific
> ECC files/errors
> > > > >>>>
> > > > >>>>      c30c40d6c63d StandaloneMmPkg: Update YAML to ignore specific
> ECC files/errors
> > > > >>>>
> > > > >>>>      9944508e85f1 ShellPkg: Update YAML to ignore specific ECC
> files/errors
> > > > >>>>
> > > > >>>>      60fa40be458d SecurityPkg: Update YAML to ignore specific ECC
> files/errors
> > > > >>>>
> > > > >>>>      df790cd6b37e MdePkg: Update YAML to ignore specific ECC
> files/errors
> > > > >>>>
> > > > >>>>      9deb9370766e MdeModulePkg: Update YAML to ignore specific
> ECC files/errors
> > > > >>>>
> > > > >>>>      d7d30e8f219f EmulatorPkg: Update YAML to ignore specific ECC
> files/errors
> > > > >>>>
> > > > >>>>      d5744ecba813 CryptoPkg: Update YAML to ignore specific ECC
> files/errors
> > > > >>>>
> > > > >>>>      c97fee87f0f9 ArmVirtPkg: Update YAML to ignore specific ECC
> files/errors
> > > > >>>>
> > > > >>>>      1939fc9569f2 ArmPlatformPkg: Update YAML to ignore specific
> ECC files/errors
> > > > >>>>
> > > > >>>>      365dced2c37a ArmPkg: Update YAML to ignore specific ECC
> files/errors
> > > > >>>>
> > > > >>>>      76a1ce4d5fec .azurepipelines/templates: Update max pipeline job
> time to 2 hours
> > > > >>>>
> > > > >>>>      99f84ff47390 .pytools/Plugin/LicenseCheck: Use temp directory
> for git diff output
> > > > >>>>
> > > > >>>>      3019f1bbabf1 .pytool/Plugin/EccCheck: Add performance
> optimizations
> > > > >>>>
> > > > >>>>      854462bd3479 .pytool/Plugin/EccCheck: Remove temp directory
> on exception
> > > > >>>>
> > > > >>>>      69877614fdee .pytool/Plugin/EccCheck: Remove RevertCode()
> > > > >>>>
> > > > >>>>      --ref1
> > > > >>>>
> > > > >>>>      ef9a059cdb15 EmulatorPkg/Win/Host: Update CC_FLAGS
> > > > >>>>
> > > > >>>>      bb1bba3d7767 (tag: edk2-stable202111) NetworkPkg: Fix invalid
> pointer for DNS response token on error
> > > > >>>>
> > > > >>>>      Best regards,
> > > > >>>>
> > > > >>>>      Mike
> > > > >>>>
> > > > >>>>
> 
> 
> 
> 


  parent reply	other threads:[~2021-12-07  3:47 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-30 22:34 Uncrustify Conversion Detailed Plan and Extended Hard Freeze Update #4 Michael D Kinney
2021-12-01  1:42 ` Wu, Hao A
2021-12-01  2:10 ` 回复: [edk2-devel] " gaoliming
2021-12-01  2:26 ` Ni, Ray
2021-12-01  2:45 ` Chiu, Chasel
2021-12-01  5:34 ` Wang, Jian J
2021-12-01  6:38 ` Wang, Jian J
2021-12-01  7:33   ` [edk2-devel] " Abner Chang
2021-12-01 16:43     ` Michael D Kinney
2021-12-01 17:05       ` Michael Kubacki
2021-12-01 17:39         ` Michael D Kinney
2021-12-02  5:48           ` Abner Chang
2021-12-02 11:00         ` Gerd Hoffmann
2021-12-02 15:33           ` Michael Kubacki
2021-12-02 16:23             ` Gerd Hoffmann
2021-12-02 17:15               ` Michael Kubacki
2021-12-02 17:33                 ` Michael D Kinney
2021-12-01 12:59 ` Sami Mujawar
2021-12-01 18:52 ` Bret Barkelew
2021-12-02 18:27 ` [edk2-devel] " Maciej Rabeda
2021-12-02 19:45   ` Michael D Kinney
2021-12-02 21:56     ` Michael Kubacki
2021-12-03  0:14       ` Michael D Kinney
2021-12-03  0:18         ` Michael D Kinney
2021-12-03  0:31           ` Michael Kubacki
2021-12-03  0:53             ` Michael D Kinney
2021-12-03  2:22               ` Michael D Kinney
2021-12-06  1:17                 ` Michael D Kinney
2021-12-06  1:23                   ` Wu, Hao A
2021-12-06  2:29                     ` Ni, Ray
2021-12-06  9:31                   ` 回复: " gaoliming
2021-12-06 19:45                   ` Bret Barkelew
2021-12-06 20:28                   ` Maciej Rabeda
2021-12-07  0:26                   ` Abner Chang
2021-12-07  3:47                   ` Wang, Jian J [this message]
2021-12-07  4:21                   ` Chiu, Chasel
2021-12-07 10:02                   ` Sami Mujawar
2021-12-03  8:56 ` Gerd Hoffmann
2021-12-03 16:25   ` Michael D Kinney

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CO1PR11MB4945272431D6C62155BEDD4BB66E9@CO1PR11MB4945.namprd11.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox