From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.byosoft.com.cn (mail.byosoft.com.cn [58.240.74.242]) by mx.groups.io with SMTP id smtpd.web08.49271.1638783069052455818 for ; Mon, 06 Dec 2021 01:31:10 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=none, err=permanent DNS error (domain: byosoft.com.cn, ip: 58.240.74.242, mailfrom: gaoliming@byosoft.com.cn) Received: from DESKTOPS6D0PVI ([58.246.60.130]) (envelope-sender ) by 192.168.6.13 with ESMTP for ; Mon, 06 Dec 2021 17:31:00 +0800 X-WM-Sender: gaoliming@byosoft.com.cn X-Originating-IP: 58.246.60.130 X-WM-AuthFlag: YES X-WM-AuthUser: gaoliming@byosoft.com.cn From: "gaoliming" To: , , "'Michael Kubacki'" , , "'Michael Kubacki'" , "'Andrew Fish'" , "'Leif Lindholm'" References: <6f929e61-6b8f-9ac5-5184-a9949c9fae75@linux.intel.com> <438f9138-641b-4973-bca3-cad990526cee@linux.microsoft.com> <0c319cd4-3726-c6bb-c31e-6693b6a18601@linux.microsoft.com> In-Reply-To: Subject: =?UTF-8?B?5Zue5aSNOiBbZWRrMi1kZXZlbF0gVW5jcnVzdGlmeSBDb252ZXJzaW9uIERldGFpbGVkIFBsYW4gYW5kIEV4dGVuZGVkIEhhcmQgRnJlZXplIFVwZGF0ZSAjNA==?= Date: Mon, 6 Dec 2021 17:31:01 +0800 Message-ID: <02c601d7ea83$fbb5ccd0$f3216670$@byosoft.com.cn> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQITSuMBO9cmHF3d1XJWTt0XshAk6gIKNBEUAvkgYPQBsIhl6gIb3KWXAauzvoECaT1ikgKTLBXgAiXRvEkAkncWU6sc8NMw Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Content-Language: zh-cn Mike: I have no comments for these changes. For MdePkg, MdeModulePkg, FmpDevice= Pkg, Reviewed-by: Liming Gao =20 Thanks Liming > -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6----- > =E5=8F=91=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io = =E4=BB=A3=E8=A1=A8 Michael D > Kinney > =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2021=E5=B9=B412=E6=9C=886=E6=97=A5 = 9:18 > =E6=94=B6=E4=BB=B6=E4=BA=BA: Michael Kubacki ; > devel@edk2.groups.io; maciej.rabeda@linux.intel.com; Michael Kubacki > ; Andrew Fish (afish@apple.com) > ; Leif Lindholm ; Kinney, Michael D > > =E4=B8=BB=E9=A2=98: Re: [edk2-devel] Uncrustify Conversion Detailed Plan = and Extended > Hard Freeze Update #4 >=20 > Hello EDK II Maintainers, >=20 > 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 fr= om a > C parsing perspective, but the EDK II usage places C statements or blocks= of > C code as the parameter to this macro. >=20 > 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. >=20 > #define DEBUG_CODE(Expression) \ > DEBUG_CODE_BEGIN (); \ > Expression \ > DEBUG_CODE_END () >=20 > 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. >=20 > 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 {} do= es > uncrustify perform incorrect formatting. >=20 > 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. >=20 > I have posted a branch with these additional patches: >=20 >=20 > https://github.com/mdkinney/edk2/tree/Bug_3737_3760_3739_ApplyUncrus > tifyChanges_V7 >=20 > I have performed CompareBuild tests with this revised patch series with > the DEBUG_CODE changes. It passes 100% showing no binary differences. >=20 > https://github.com/mdkinney/edk2/actions/runs/1542454606 >=20 > I have opened a PR to run this patch series through EDK II CI. It also pa= sses > 100%. >=20 > https://github.com/tianocore/edk2/pull/2236 >=20 > The summary of changes made since the V6 review are: >=20 > 1) Change uncrustify configuration assignment alignment threshold to = 0 >=20 > align_assign_thresh =3D 0 >=20 > 2) Replace ", OPTIONAL" with " OPTIONAL," >=20 > 3) Replace DEBUG_CODE(Expression) with >=20 > DEBUG_CODE_BEGIN(); > Expression > DEBUG_CODE_END() >=20 > if Expression is complex and contains braces {}. >=20 > 4) No changes to uncrustify tool required. >=20 > Please review the differences between the following 2 branches and provid= e > feedback or a Series Reviewed-by if you agree with these additional chang= es. >=20 >=20 > https://github.com/mdkinney/edk2/tree/Bug_3737_3760_3739_ApplyUncrus > tifyChanges_V6 >=20 > https://github.com/mdkinney/edk2/tree/Bug_3737_3760_3739_ApplyUncrus > tifyChanges_V7 >=20 > The goal is to complete the review and get the uncrustify change committe= d > tomorrow so the extended hard freeze can be lifted. >=20 > Thanks, >=20 > Mike >=20 > > -----Original Message----- > > From: Kinney, Michael D > > Sent: Thursday, December 2, 2021 6:23 PM > > To: Michael Kubacki ; > devel@edk2.groups.io; maciej.rabeda@linux.intel.com; Michael Kubacki > > ; Andrew Fish (afish@apple.com) > ; Leif Lindholm ; > > Kinney, Michael D > > Subject: RE: [edk2-devel] Uncrustify Conversion Detailed Plan and Exten= ded > 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 ch= ange > for assignment alignment. > > > > > https://github.com/mdkinney/edk2/tree/Bug_3737_3760_3739_ApplyUncrus > tifyChanges_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 w= ill > generate a V7. > > > > Thanks, > > > > Mike > > > > > -----Original Message----- > > > From: Kinney, Michael D > > > Sent: Thursday, December 2, 2021 4:53 PM > > > To: Michael Kubacki ; > devel@edk2.groups.io; maciej.rabeda@linux.intel.com; Michael Kubacki > > > ; Andrew Fish (afish@apple.com) > ; Leif Lindholm ; > > > Kinney, Michael D > > > 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 > > > > Sent: Thursday, December 2, 2021 4:31 PM > > > > To: Kinney, Michael D ; > devel@edk2.groups.io; maciej.rabeda@linux.intel.com; Michael > > Kubacki > > > > ; Andrew Fish (afish@apple.com) > ; Leif Lindholm > > > > 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 Uncrusti= fy > > > > tool update by tomorrow for (4). > > > > > > > > That will require an update in uncrustify_ext_dep.yaml to pull in t= he > > > > new version. I'm assuming I should also update uncrustify.cfg to se= t > > > > align_assign_thresh =3D 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 alo= ng with > the recommendations. > > > > > > > > > > 1) Exclusion feature for UncrustifyCheck. There are 2 directorie= s > 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 sam= e > 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 appea= rs > 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=3D 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 paramet= er > 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 tha= t > 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 ex= tra > set of braces {}. > > > > > > > > > > > > > > > I have posted a branch for testing purposes that implements (2) a= nd > (3). > > > > > > > > > > Branch: > https://github.com/mdkinney/edk2/tree/TestOnly_Bug_3737_3739_ApplyUn > crustifyChanges_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_ApplyUncrustifyCh > anges_V5 > > > > > > https://github.com/mdkinney/edk2/tree/TestOnly_Bug_3737_3739_ApplyUn > crustifyChanges_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 > > > > >> Sent: Thursday, December 2, 2021 4:15 PM > > > > >> To: Michael Kubacki ; > devel@edk2.groups.io; maciej.rabeda@linux.intel.com; Michael > > > > Kubacki > > > > >> ; Andrew Fish (afish@apple.com) > ; Leif Lindholm > > ; > > > > >> Kinney, Michael D > > > > >> Subject: RE: [edk2-devel] Uncrustify Conversion Detailed Plan an= d > Extended Hard Freeze Update #4 > > > > >> > > > > >> Hi Michael, > > > > >> > > > > >> Reponses inline below. > > > > >> > > > > >> I would like to summarize the 4 issues raised in the past day al= ong > with the recommendations. > > > > >> > > > > >> 1) Exclusion feature for UncrustifyCheck. There are 2 directori= es > 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 requir= e > all developers to make > > > > >> sure their method of using uncrustify also excludes those sa= me > areas. This requires > > > > >> extra steps for all developers and maintainers. > > > > >> > > > > >> If we do not add the exclusion feature, then the 8 files wil= l > require an extra step > > > > >> to sync with the original source of those files. The rate o= f > 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 exclusion= s > increases significantly. > > > > >> > > > > >> 2) Alignment of assignments. The threshold of 4 characters appe= ars > 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=3D 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 parame= ter > 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 th= at > needs to be formatted. > > > > >> Complex blocks with if/while/for/case statements are impacte= d > 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_ApplyUn > crustifyChanges_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_ApplyUncrustifyCh > anges_V5 > > > > >> > https://github.com/mdkinney/edk2/tree/TestOnly_Bug_3737_3739_ApplyUn > crustifyChanges_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 > > > > >>> Sent: Thursday, December 2, 2021 1:57 PM > > > > >>> To: devel@edk2.groups.io; Kinney, Michael D > ; maciej.rabeda@linux.intel.com; Michael > > > > Kubacki > > > > >>> ; Andrew Fish (afish@apple.com) > ; Leif Lindholm > > > > > > >>> Subject: Re: [edk2-devel] Uncrustify Conversion Detailed Plan a= nd > 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 '=3D' in assignments. > > > > >>>> > > > > >>>> # Use a negative number for absolute thresholds. > > > > >>>> > > > > >>>> # > > > > >>>> > > > > >>>> # 0: No limit (default). > > > > >>>> > > > > >>>> align_assign_thresh=3D 0# number > > > > >>>> > > > > >>>> The edk2 setting for this is: > > > > >>>> > > > > >>>> align_assign_thresh=3D 4 > > > > >>>> > > > > >>>> This means blocks of assignments that are different than more > than 4 > > > > >>>> spaces will be considered a new block. > > > > >>>> > > > > >>>> =E2=80=98HwAddreLen=E2=80=99 and =E2=80=98Xid=E2=80=99 are mor= e than 4 characters in length > > > > >>>> different.Same for =E2=80=98Xid=E2=80=99 and =E2=80=98Reserved= =E2=80=99.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 =E2=80=98=3D=E2=80=99 are al= igned. > > > > >>>> > > > > >>>> Is there a reason =E2=80=984=E2=80=99 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 an= d > every > > > > >>> case is difficult to cover well. Please feel free to suggest an= y > changes. > > > > >>> > > > > >> > > > > >> I recommend we use the default value of 0. That way, any code t= hat > choose to > > > > >> align assignments will still be aligned. If a developer does no= t 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 > /configuration.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 t= o > 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 fo= rk > since > > > > >>> DEBUG_CODE() is being treated as a macro function call and code > blocks > > > > >>> are not expected there. In addition, some special treatment mig= ht > be > > > > >>> needed for alignment in between > DEBUG_CODE_BEGIN()/DEBUG_CODE_END(). > > > > >>> > > > > >>> I'm happy to look at this. However, regression validation can t= ake 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 wi= thout > > > > >> any if/while/for/case statements that require further indent, th= en 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 th= e > function > > > > >>>> declaration. > > > > >>>> > > > > >>>> TYPE ParamName OPTIONAL, > > > > >>>> > > > > >>>> TYPEParamName, OPTIONAL > > > > >>>> > > > > >>>> TYPEParamName OPTIONAL > > > > >>>> > > > > >>>> OPTIONAL is defined to nothing in edk2 builds.From a uncrustif= y > > > > >>>> perspective, we really want is to be > > > > >>>> > > > > >>>> ignored or more correctly treated it as a token that is attach= ed 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 consist= ent > pattern? > > > > >>> > > > > >>>> Thanks, > > > > >>>> > > > > >>>> Mike > > > > >>>> > > > > >>>> *From:*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 Kubacki > > > > >>>> ; Andrew Fish > (afish@apple.com) > > > > >>>> ; Leif Lindholm > > > > >>>> *Subject:* Re: [edk2-devel] Uncrustify Conversion Detailed Pla= n > and > > > > >>>> Extended Hard Freeze Update #4 > > > > >>>> > > > > >>>> Hey Mike, > > > > >>>> > > > > >>>> While most of the changes related to fixing coding style viola= tions, > > > > >>>> there are a couple of changes to NetworkPkg in that PR that ma= ke > 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 an= d > CompareBuild > > > > >>>> > > > > >>>> tool functionality and results and you accept the source > style > > > > >>>> > > > > >>>> changes to your package. > > > > >>>> > > > > >>>> > *https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrustifyC > hanges_V5 > > > > >>> > hanges_V5> > > > > >>>> > > > > >>>> *https://github.com/tianocore/edk2/pull/2229 > > > > > >>>> > > > > >>>> > *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 no= w. > > > > >>>> > > > > >>>> * 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 uncru= stify > 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 li= ke > 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 > _v6 > > > > >>> > n_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 matc= h > 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 th= e > 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 > > > > >>>> > > > > >>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > > > >>>> > > > > >>>> * Sources:https://dev.azure.com/projectmu/_git/Uncrustify > > > > > >>>> > > > > >>>> * > Documentation:https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Un > crustify.wiki/1/Project-Mu-(EDK- > > II)- > > > > Fork- > > > > >>> Readme > Project-Mu-(EDK-II)-Fork-Readme> > > > > >>>> > > > > >>>> * > Download:https://dev.azure.com/projectmu/Uncrustify/_packaging?_a=3Dpack > age&feed=3Dmu_uncrustify&package=3Dmu- > > > > >>> uncrustify-release&protocolType=3DNuGet&version=3D73.0.3 > > > > >>> > =3Dmu_uncrustify&package=3Dmu-uncrustify- > > > > >>> release&protocolType=3DNuGet&version=3D73.0.3> > > > > >>>> > > > > >>>> Installing Uncrustify > > > > >>>> > > > > >>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D > > > > >>>> > > > > >>>> 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 > > > > >>> > > > > > >>>> > > > > >>>> 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 > > > > >>>> > > > > >>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > > > >>>> > > > > >>>> Once the uncrustify checker is active in EDK II CI, > developers must > > > > >>>> > > > > >>>> make sure their patches are run through the uncrustify to= ol > before > > > > >>>> > > > > >>>> sending the patches for review. > > > > >>>> > > > > >>>> Developers must install and run uncrustify against change= s > files before > > > > >>>> > > > > >>>> sending patch review emails or submitting PR for EDK II C= I.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 > > > > >>>> > > > > >>>> > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D > > > > >>>> > > > > >>>> 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_ApplyUncrustifyCh > anges_V5 > > > > >>> > hanges_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 w= hen > 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 th= e > 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 > > > > >>>> > > > > >>>> > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D > > > > >>>> > > > > >>>> Tools the view file and line history do work with the lar= ge > uncrustify > > > > >>>> > > > > >>>> patch series.One impact is that the operations can be ver= y > slow due > > > > >>>> > > > > >>>> to the large uncrustify patches. > > > > >>>> > > > > >>>> One option to provide a faster experience is to provide a= n > alternate > > > > >>>> > > > > >>>> version of the edk2 repository as "documentation" that ha= s > the > > > > >>>> > > > > >>>> entire history re-written with uncrustify run on every > commit. > > > > >>>> > > > > >>>> The tool called git-filter-repo can be used to perform th= is > > > > >>>> > > > > >>>> 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/blob/main/contrib/filter-repo-d= em > os/lint-history > > > > >>> > mos/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 > edk2-uncrustified > > > > >>>> > > > > >>>> cd edk2-uncrustified > > > > >>>> > > > > >>>> 2) Setup python virtual env, install pytools, and run stu= art > commands > > > > >>>> > > > > >>>> to setup build environment which includes installing > uncrustify tools. > > > > >>>> > > > > >>>> > 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 w= hen > 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/blob/main/contrib/filter-repo-d= em > os/lint-history > > > > >>> > mos/lint-history> > > > > >>>> > > > > >>>> Line #127 - Add try except around subprocess.check_call() > with except > > > > >>>> > > > > >>>> being pass.This is required because there are a few commi= ts > of C > > > > >>>> > > > > >>>> files in the edk2 repo that have incorrect C syntax and d= o > not > > > > >>>> > > > > >>>> build with a C compiler and break the uncrustify parser.S= kip > reformat > > > > >>>> > > > > >>>> of C files that can not be parsed by uncrustify.These rar= e > instances > > > > >>>> > > > > >>>> are addressed in the commit that fixes the C syntax error= . > > > > >>>> > > > > >>>> Run this slightly modified version of lint-history.Includ= e 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.c= fg > --replace --no-backup --if-changed > > > > >>>> > > > > >>>> Order of PRs to apply during extended hard freeze > > > > >>>> > > > > >>>> > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D > > > > >>>> > > > > >>>> 1) Update EmulatorPkg Win Host [BuildOptions] MSFT > CC_FLAGS to not force debug information > > > > >>>> > > > > >>>> *https://bugzilla.tianocore.org/show_bug.cgi?id=3D3747 > > > > > >>>> > > > > >>>> > *https://github.com/mdkinney/edk2/tree/Bug_3747_EmulatorPkg_WinHost_ > ReproducibleBuild > > > > >>> > ReproducibleBuild> > > > > >>>> > > > > >>>> *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=3D2986 > > > > > >>>> > > > > >>>> > *https://github.com/mdkinney/edk2/tree/Bug_2986_EccCheckRemoveGitRe > vert_V2 > > > > >>> > vert_V2> > > > > >>>> > > > > >>>> *https://github.com/tianocore/edk2/pull/2216 > > > > > >>>> > > > > >>>> * Required for EDK II CI to complete in a reasonable peri= od > of time when > > > > >>>> > > > > >>>> processing the 4000+ source file style changes made by > uncrustify. > > > > >>>> > > > > >>>> * Also fixes critical bugs that can potentially corrupt g= it state > when > > > > >>>> > > > > >>>> EccCheck is run locally. > > > > >>>> > > > > >>>> * Status: Review complete.PR pushed. > > > > >>>> > > > > >>>> 3) Update pytool LicenseCheck plugin to use temp director= y > for diff output file > > > > >>>> > > > > >>>> *https://bugzilla.tianocore.org/show_bug.cgi?id=3D3746 > > > > > >>>> > > > > >>>> > *https://github.com/mdkinney/edk2/tree/Bug_3746_LicenseCheckUseDiffOu > tputFile_V2 > > > > >>> > tputFile_V2> > > > > >>>> > > > > >>>> *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=3D3750 > > > > > >>>> > > > > >>>> > *https://github.com/mdkinney/edk2/tree/Bug_3750_IncreaseAzurePipelines > Timeout > > > > >>> > Timeout> > > > > >>>> > > > > >>>> *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/error= s > > > > >>>> > > > > >>>> *https://bugzilla.tianocore.org/show_bug.cgi?id=3D3749 > > > > > >>>> > > > > >>>> > *https://github.com/mdkinney/edk2/tree/Bug_3749_EccCheckIgnoreFilesErr > ors > > > > >>> > ors> > > > > >>>> > > > > >>>> *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=3D3737 > > > > > >>>> > > > > >>>> *https://bugzilla.tianocore.org/show_bug.cgi?id=3D3739 > > > > > >>>> > > > > >>>> > *https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrustifyC > hanges_V5 > > > > >>> > hanges_V5> > > > > >>>> > > > > >>>> *https://github.com/tianocore/edk2/pull/2229 > > > > > >>>> > > > > >>>> * Build comparison result > PASS: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=3D3748 > > > > > >>>> > > > > >>>> > *https://github.com/mdkinney/edk2/tree/Bug_3748_add_uncrustify_ci_plugi > n_v6 > > > > >>> > n_v6> > > > > >>>> > > > > >>>> * Required to enforce all PRs submitted to EDK II CI matc= h > 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 > > > > >>>> > > > > >>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > > > >>>> > > > > >>>> * Build Comparison results must pass 100% across the full > set of PRs before > > > > >>>> > > > > >>>> the individual PRs can be pushed in the order listed abov= e. > > > > >>>> > > > > >>>> * > Branch:https://github.com/mdkinney/edk2/tree/TestOnly_Uncrustify_PR_Ser > ies > > > > >>> > > > > > >>>> > > > > >>>> * PR:https://github.com/tianocore/edk2/pull/2229 > > > > > >>>> > > > > >>>> Status =3D PASS > > > > >>>> > > > > >>>> * CompareBuild: > > > > >>>> > > > > >>>> > Branch:https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncr > ustifyChanges_V5 > > > > >>> > hanges_V5> > > > > >>>> > > > > >>>> --ref1:ef9a059cdb15844fe52a49af2bf7d86b9dd3e9bf > > > > >>>> > > > > >>>> --ref2:Bug_3737_3739_ApplyUncrustifyChanges_V5 > > > > >>>> > > > > >>>> Extra Options: -n 4 --quiet > > > > >>>> > > > > >>>> > Results: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 --ref= 1 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 > > > > >>>> > > > > >>>> >=20 >=20 >=20 >=20