Laszlo and Leif: Thanks for your detail review. I will continue to monitor the coming changes for 201911 stable tag.
Thanks Liming > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Leif Lindholm > Sent: Wednesday, November 20, 2019 3:02 AM > To: Laszlo Ersek <ler...@redhat.com> > Cc: Gao, Liming <liming....@intel.com>; Kinney, Michael D > <michael.d.kin...@intel.com>; 'af...@apple.com' <af...@apple.com>; > devel@edk2.groups.io > Subject: Re: [edk2-devel] Patch List for 201911 stable tag > > On Tue, Nov 19, 2019 at 06:50:19PM +0100, Laszlo Ersek wrote: > > On 11/19/19 15:25, Gao, Liming wrote: > > > Hi Stewards and all: > > > I collect current patch lists in devel mail list. Those patch > > > contributors request to add them for 201911 stable tag. Because the > > > time is close to Hard 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/50602 [PATCH V2] BaseTools: Add > > > [packages] section in dsc file > > > > This patch can be merged during the Soft Feature Freeze. It was posted > > before the Soft Feature Freeze, and also reviewed (by Bob, i.e. a > > BaseTools Maintainer) before the Soft Feature Freeze. > > > > As far as I can see, there is still an outstanding question from you, to > > Zhiju ("Can you show what test are done for this new support?"), so I > > think we should await the response to that. > > > > Note that the patch should not be merged once the Hard Feature Freeze > > starts, so there are ~3 days for Zhiju to answer the question about > > testing (and for you to acknowledge that you are OK with the reply). > > Agreed. > > > > Bug List (those all have pass code review): > > > https://edk2.groups.io/g/devel/message/50625 [PATCH v1] > > > MdeModulePkg/NvmExpressDxe: Fix wrong queue size for async IO > queues > > > > Looks very much like a bugfix to me, so it's suitable for merging even > > during the Hard Feature Freeze. > > I agree. But I am still slightly nervous about changing such a > fundamental part of such a fundamental driver. Certainly if it is > going in, I want it in ASAP, not just at the end of soft freeze - to > give us as much time as possible to revert it if the fix exposes > latent errors in previously working systems. > > > > https://edk2.groups.io/g/devel/message/50406 [PATCH 1/1] MdePkg/Include: > > > Add missing definitions of SMBIOS type 42h in > SmBios.h > > > > Based on Abner's response in the thread, this change does not appear > > necessary for fixing actual functionality bugs; it rather completes a > > previously incomplete feature addition. And Abner is not in a rush to > > catch the upcoming stable tag with the patch. I suggest to delay it. > > > > If others disagree, I won't insist; the above is just my preference. > > I'm OK either way. > > > > https://edk2.groups.io/g/devel/message/50661 [PATCH] UefiCpuPkg: Update > > > the coding styles > > > > Hmmm, quite undecided on this one. Does not fix a functionality bug > > either, but what it fixes *are* a coding style bugs, and the patch is > > low risk. I'm leaning towards merging it. > > I am against merging this, even though it's low-risk. > > The process says: > "By the date of the soft feature freeze, developers must have sent > their patches to the mailing list and received positive maintainer > reviews (Reviewed-by or Acked-by tags)." > This received Acks 4 days late. > > If it came with a commit message indicating the incorrect comment > syntax caused problems with document generation, then maybe it could > be considered from a bugfix standpoint. But it didn't and it's too > late to re-scope the change at this point. > > I also dislike the mixing of doxygen formating changes and plain > whitespace changes. Even though trivial, it ought to be split up. > > > > https://edk2.groups.io/g/devel/message/50662 [PATCH] MdePkg: Update the > > > comments of IsLanguageSupported > > > > This was even reviewed by a package maintainer (= you) before the SFF, > > so it can definitely go in. > > Agree (if cutting it close). > > > > https://edk2.groups.io/g/devel/message/50663 [PATCH 0/3] Add missing > > > strings for uni files > > > > First of all, the structure of this series is wrong; please see my > > feedback here: > > > > https://edk2.groups.io/g/devel/message/50666 > > > > (The two patches discussed just above were incorrectly included in the > > same posting.) > > > > Second, the three patches for the UNI files add too much brand new text > > for my taste, for them to be considered bugfixes. The patches were > > posted in time for the SFF, but the maintainer reviews came too late: > > > > https://edk2.groups.io/g/devel/message/50872 > > https://edk2.groups.io/g/devel/message/50869 > > https://edk2.groups.io/g/devel/message/50870 > > > > I suggest postponing. > > Agree. > > > > https://edk2.groups.io/g/devel/message/50866 [PATCH V1 0/2] Improve > > > PeiInstallPeiMemory() description > > > > I'm seriously confused by the subject prefixes in this patch thread. > > What's going on with the version numbers? > > > > [edk2-devel] [PATCH V1 0/2] Improve PeiInstallPeiMemory() description > > [edk2-devel] [PATCH V3 1/2] MdeModulePkg PeiCore: Improve > > PeiInstallPeiMemory() description > > [edk2-devel] [PATCH V1 2/2] MdePkg PiPeiCis.h: Improve > > PeiInstallPeiMemory() description > > > > Other than that... I'm torn. I guess I could be convinced that these > > patches are indeed bugfixes, so I'm leaning towards merging them. > > Non-functional change submitted after start of soft-freeze? > I don't see why it should be considered. > > > > https://edk2.groups.io/g/devel/message/50841 [PATCH V2 1/1] MdeModulePkg > > > PeiCore: Fix typos > > > > Personally I'm not happy about this patch. It's way too large for my taste: > > > > MdeModulePkg/Core/Pei/PeiMain.inf | 10 ++-- > > MdeModulePkg/Core/Pei/FwVol/FwVol.h | 20 +++---- > > MdeModulePkg/Core/Pei/PeiMain.h | 52 ++++++++-------- > > MdeModulePkg/Core/Pei/Dependency/Dependency.c | 12 ++-- > > MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c | 51 ++++++++-------- > > MdeModulePkg/Core/Pei/FwVol/FwVol.c | 63 ++++++++++---------- > > MdeModulePkg/Core/Pei/Hob/Hob.c | 4 +- > > MdeModulePkg/Core/Pei/Image/Image.c | 10 ++-- > > MdeModulePkg/Core/Pei/Memory/MemoryServices.c | 18 +++--- > > MdeModulePkg/Core/Pei/PeiMain/PeiMain.c | 2 +- > > MdeModulePkg/Core/Pei/Ppi/Ppi.c | 4 +- > > MdeModulePkg/Core/Pei/Security/Security.c | 12 ++-- > > 12 files changed, 129 insertions(+), 129 deletions(-) > > > > and it mixes multiple kinds of changes: > > > > "Fixes typos and clarifies some wording throughout PeiCore." > > > > When reviewing such a patch, the reviewer has a difficult time telling > > apart purely syntactic (typo) fixes from semantic (wording) fixes. As a > > reviewer I would suggest splitting this patch at least in two (typos vs. > > semantics). Then I could be convinced such a set of two patches is > > purely a bugfix. > > > > I'm leaning towards "postpone" on this one, but I can see why people > > would think "that's arbitrary". I guess I'll have to defer to others in > > this instance. > > Non-functional change submitted after start of soft-freeze? > I don't see why it should be considered. > > I also agree on the needs splitting up bit. > > Best Regards, > > Leif > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#50938): https://edk2.groups.io/g/devel/message/50938 Mute This Topic: https://groups.io/mt/60556595/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
