From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.120]) by mx.groups.io with SMTP id smtpd.web11.12.1574185828795933612 for ; Tue, 19 Nov 2019 09:50:29 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=QapQd8Ue; spf=pass (domain: redhat.com, ip: 207.211.31.120, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1574185827; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=P88p5u5Td/097uHARfsrBPrJ6twFKJaHX6VxdlRGsLk=; b=QapQd8UeZv3+yfoKRKBBE1inh34GODuUbFD130iaTxAF7lSJOWC21g8HUiHnJjUjld5y0e j57OtUTJgmqw7o7MWiGsfl6nTw18YHq5m2/8MVliPZMt5PvZ16yW+lZtbIJAcD5k3g8Q5u QtPMRDJDvqpk4EbJ3yMPHY/XpilskzA= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-387-ZzRnub48NYK2sG-g7CYLxw-1; Tue, 19 Nov 2019 12:50:23 -0500 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 35904107ACC4; Tue, 19 Nov 2019 17:50:22 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-47.ams2.redhat.com [10.36.116.47]) by smtp.corp.redhat.com (Postfix) with ESMTP id D0AD160BE1; Tue, 19 Nov 2019 17:50:20 +0000 (UTC) Subject: Re: Patch List for 201911 stable tag To: "Gao, Liming" , "'leif.lindholm@linaro.org'" , "Kinney, Michael D" , "'afish@apple.com'" Cc: "devel@edk2.groups.io" References: <4A89E2EF3DFEDB4C8BFDE51014F606A14E5437BA@SHSMSX104.ccr.corp.intel.com> From: "Laszlo Ersek" Message-ID: <48a7f95e-1028-ea52-9980-da7af871cef2@redhat.com> Date: Tue, 19 Nov 2019 18:50:19 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <4A89E2EF3DFEDB4C8BFDE51014F606A14E5437BA@SHSMSX104.ccr.corp.intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-MC-Unique: ZzRnub48NYK2sG-g7CYLxw-1 X-Mimecast-Spam-Score: 0 Content-Language: en-US Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable 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 [p= ackages] 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). > Bug List (those all have pass code review): > https://edk2.groups.io/g/devel/message/50625 [PATCH v1] MdeModulePkg/NvmE= xpressDxe: 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. > 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. > https://edk2.groups.io/g/devel/message/50661 [PATCH] UefiCpuPkg: Update t= he 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. > https://edk2.groups.io/g/devel/message/50662 [PATCH] MdePkg: Update the c= omments of IsLanguageSupported This was even reviewed by a package maintainer (=3D you) before the SFF, so it can definitely go in. > https://edk2.groups.io/g/devel/message/50663 [PATCH 0/3] Add missing stri= ngs 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. > https://edk2.groups.io/g/devel/message/50866 [PATCH V1 0/2] Improve PeiIn= stallPeiMemory() 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 PeiInstallPeiMe= mory() description [edk2-devel] [PATCH V1 2/2] MdePkg PiPeiCis.h: Improve PeiInstallPeiMemor= y() 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. > 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. Thanks Laszlo