Re: Patch List for 201908 stable tag

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

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Friday, August 16, 2019 11:38 AM
> To: Gao, Liming <liming.gao@intel.com>;
> leif.lindholm@linaro.org; Kinney, Michael D
> <michael.d.kinney@intel.com>; afish@apple.com; Cetola,
> Stephano <stephano.cetola@intel.com>
> Cc: devel@edk2.groups.io; Shi, Steven
> <steven.shi@intel.com>; Kuo, Donald
> <donald.kuo@intel.com>; Vitaly Cheptsov
> <vit9696@protonmail.com>; Dong, Eric
> <eric.dong@intel.com>
> Subject: Re: Patch List for 201908 stable tag
> 
> On 08/16/19 10:00, Gao, Liming wrote:
> >
> > From: Gao, Liming [mailto:liming.gao@intel.com]
> > Sent: Friday, August 16, 2019 3:59 PM
> > To: Laszlo Ersek (lersek@redhat.com)
> <lersek@redhat.com>;
> > leif.lindholm@linaro.org; Kinney, Michael D
> > <michael.d.kinney@intel.com>; afish@apple.com; Cetola,
> Stephano
> > <stephano.cetola@intel.com>
> > Cc: edk2-devel@lists.01.org
> > Subject: Patch List for 201908 stable tag
> >
> > Hi Stewards and all:
> >   I collect current patch lists in devel mail list.
> Those patch contributors request to add them for 201908
> stable tag. Because the time is very close to Soft
> Feature Freeze, I want to collect your feedback for
> below patches.
> >
> > Feature List (those all have pass code review):
> > https://edk2.groups.io/g/devel/message/45734 [PATCH v6
> 0/5] Build
> > cache enhancement
> 
> Seems to be well documented (both in commit messages and
> in the BZ), and the series was approved by a BaseTools
> maintainer [1] before the soft feature freeze [2].
> 
> [1] https://edk2.groups.io/g/devel/message/45783
> [2] https://edk2.groups.io/g/devel/message/45806
> 
> The series can be pushed during the soft feature freeze,
> according to the current SFF definition.
> 
> Some requests regarding the bugzilla:
> 
> - Comment 0 in the bugzilla says "redesign the platform
> hash algorithm".
> I feel that would be better reflected if we changed the
> Product field to TianoCore Feature Requests.
> 
> - Please capture the v1..v6 cover letters, with mailing
> list archive links, in the bugzilla.
> 
> > https://edk2.groups.io/g/devel/message/45707 [PATCH]
> UefiCpuPkg:
> > Adding a new TSC library by using CPUID(0x15) TSC leaf
> 
> Accepted by a UefiCpuPkg maintainer [3] before the SFF
> [2].
> 
> [3] https://edk2.groups.io/g/devel/message/45787
> 
> Can be pushed during the soft feature freeze, according
> to the current SFF definition.
> 
> According to my comments sent earlier today for this
> series, the documentation and the bugzilla status are
> extremely lacking. I'm OK with pushing the series, but
> those aspects should be fixed in the bugzilla.
> Please see
> <https://edk2.groups.io/g/devel/message/45826>.
> 
> In addition, it appears multiple versions of the patch
> have been sent, without using "vN" identifiers in the
> subject prefix. Please collect all versions of the patch
> series from the mailing list archive, in chronological
> order, and link them all into the bugzilla ticket.
> 
> > https://edk2.groups.io/g/devel/message/45503 [PATCH v2
> 1/1] MdePkg:
> > Add STATIC_ASSERT macro
> 
> I commented on this today, as well:
> <https://edk2.groups.io/g/devel/message/45827>
> 
> I disagree with the maintainer approval for this patch,
> from a reviewer perspective. I think the submission
> isn't complete enough / mature enough at this stage. The
> first version of the patch was posted on 12 Aug
> <https://edk2.groups.io/g/devel/message/45451>, as far
> as I can see.
> Why are we in a mortal hurry to add a central macro
> without any call sites in edk2 proper?
> 
> Anyway I'm not going to NACK the patch. If Mike is fine
> with the patch as-is, I'll live with that, with my
> disagreement noted.

I see there is another version sent out on Friday that includes
the usage.  We do need to make sure all the compiler we use
are compatible with this macro.  The one that may be a challenge
is EBC.

I think this is a valuable new feature, but I have not see 
The critical reason why this has to go in for the 201908
stable tag.  I would rather see it as one of the first 
item after the stable tag so it can get lots of flight 
time.

> 
> > Bug List:
> > https://edk2.groups.io/g/devel/message/45794  [PATCH
> 1/1] CryptoPkg:
> > Fix coding style
> 
> Purely from the subject line, looks like a bugfix, so no
> conflict with the *soft* feature freeze.

I agree.  Bug fix for 201908 stable tag.

> 
> > https://edk2.groups.io/g/devel/message/45791 [PATCH v2
> 1/1]
> > ShellPkg/UefiShellAcpiViewCommandLib: Replace shift
> logical left
> 
> Clearly a bugfix, so ditto.

I agree.  Bug fix for 201908 stable tag.

> 
> > https://edk2.groups.io/g/devel/message/45793 [Patch
> 1/1] BaseTools:
> > Fixed issue of incorrect Module Unique Name
> 
> Ditto.

I agree.  Bug fix for 201908 stable tag.

> 
> > https://edk2.groups.io/g/devel/message/45773 [Patch v4
> 0/6] Add "test
> > then write" mechanism
> 
> Sigh, this is a messy question.
> 
> To my eyes, this introduces a feature. The cover letter
> says "add ...
> mechanism". Even though it is used to fix a bug, it
> still introduces a new facility, in my opinion. So here
> I disagree with
> <https://edk2.groups.io/g/devel/message/45590>.
> 
> For UefiCpuPkg, Ray and Eric are Maintainers, and I'm a
> reviewer. The submitter is Eric, and Ray is away. Eric
> asked me to review
> <https://edk2.groups.io/g/devel/message/45489>.
> 
> I reviewed v2. Then v3 was posted (not sure in response
> to what, as I can't see any comments under v2 that
> requested the v3 changes). V3 received a bunch of
> comments, so v4 was posted quickly afterwards.
> 
> The posting of v4 was still in time for the SFF, but I
> only arrived at it after the SFF. (And Ray is still
> away.) Technically speaking, in my book, this series has
> missed the stable tag. But that's not the submitter's
> fault, arguably.
> 
> On this, I am undecided. I will defer to the other
> stewards. If they are fine with the series going in, I'm
> OK too.

Ray is back online has have provided his review.
I am fine with this going in.  I want to see this bug fixed.
In the 201908 stable tag.

> 
> > https://edk2.groups.io/g/devel/message/45317 [Patch]
> MdeModulePkg
> > DxeCore: Fix for missing MAT update
> 
> This is a bugfix and therefore there is no conflict with
> the soft feature freeze. Jian provided a maintainer R-b,
> and I asked for a comment style cleanup and some
> documentation (commit message) improvements. In
> addition, perhaps we should conditionalize the logic on
> the gEfiMemoryTypeInformationGuid GUID HOB. (The
> discussion between Liming and myself on where the HOB
> can come from is irrelevant in that regard -- wherever
> the HOB comes from, it is suitable for checking in the
> DXE core, I think.)
> 
> I think it's fine to submit a v2 and merge it during the
> soft feature freeze (again, bugfix, so no conflict). I
> may not be around to review v2, so don't wait for my
> feedback.

I agree that this is a bug fix and should be included in
the 201908 stable tag.

> 
> Thanks
> Laszlo