Laszlo: > -----Original Message----- > From: Laszlo Ersek [mailto:ler...@redhat.com] > Sent: Wednesday, May 22, 2019 5:01 AM > To: devel@edk2.groups.io > Cc: Gao, Liming <liming....@intel.com>; Kinney, Michael D > <michael.d.kin...@intel.com>; Cetola, Stephano > <stephano.cet...@intel.com>; Andrew Fish <af...@apple.com>; Leif Lindholm > <leif.lindh...@linaro.org> > Subject: Re: [edk2-devel] [edk2] Soft Feature Freeze starts today for > edk2-stable201905 > > Hi, > > On 05/17/19 10:56, Liming Gao wrote: > > Hi, all > > > > https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning > > lists edk2-stable201905 tag planning. Now, we enter into Soft Feature > > Freeze phase. In this phase, the feature without Reviewed-by or > > Acked-by tags will be delayed after the upcoming stable tag. The patch > > review can continue without break. Below is edk2-stable201905 tag > > planning. > > > > Date (00:00:00 UTC-8) Description > > 2018-03-08 (12PM) Beginning of development > > 2019-05-17 Soft Feature Freeze > > 2019-05-24 Hard Feature Freeze > > 2019-05-31 Release > > We need to establish better enforcement for the soft feature freeze, or > else, we need to redefine what the soft feature freeze means. > > According to the current definition, and schedule, a patch that is not a > bugfix may be committed after 2019-05-17 08:00 UTC only if it has > received sufficient Acked-by / Reviewed-by tags on the list before the > exact same timestamp (2019-05-17 08:00 UTC). > > > https://github.com/lersek/edk2/wiki/SoftFeatureFreeze#what-is-the-soft-feature-freeze > > > The soft feature freeze is the beginning of the stabilization phase of > > edk2's development process. 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 means that features, and in particular non-trivial ones, must > > have been accepted by maintainers before the soft freeze date. > > > > Between the soft feature freeze and the hard feature freeze, > > previously reviewed and unit-tested features may be applied (or > > merged) to the master branch, for integration testing. Feature > > addition or enablement patches posted *or* reviewed after the soft > > feature freeze will be delayed until after the upcoming stable tag. > > So let's check the recent commit history... > > git log --oneline --reverse --since='2019-05-17T08:00:00+00:00' > > > 8da8daafc905 ShellPkg: acpiview: Add GT Frame Number validation to GTDT > > parser > > Reviewed on 2019-05-17, at 00:08 UTC: > > > 3CE959C139B4C44DBEA1810E3AA6F9000B7D374D@SHSMSX101.ccr.corp.intel.com">http://mid.mail-archive.com/3CE959C139B4C44DBEA1810E3AA6F9000B7D374D@SHSMSX101.ccr.corp.intel.com > > Valid commit. > > > > 1887b995a359 ShellPkg/UefiShellAcpiViewCommandLib: Fix PPTT cache > > attributes validation > > This patch received multiple R-b's, in time, including one from a > designated ShellPkg Reviewer: > > > DB6PR0802MB237582494362A29FEBE0DD3E84330@DB6PR0802MB2375.eurprd08.prod.outlook.com">http://mid.mail-archive.com/DB6PR0802MB237582494362A29FEBE0DD3E84330@DB6PR0802MB2375.eurprd08.prod.outlook.com > > 3C0D5C461C9E904E8F62152F6274C0BB40BD4110@SHSMSX104.ccr.corp.intel.com">http://mid.mail-archive.com/3C0D5C461C9E904E8F62152F6274C0BB40BD4110@SHSMSX104.ccr.corp.intel.com > > 3CE959C139B4C44DBEA1810E3AA6F9000B7D3832@SHSMSX101.ccr.corp.intel.com">http://mid.mail-archive.com/3CE959C139B4C44DBEA1810E3AA6F9000B7D3832@SHSMSX101.ccr.corp.intel.com > > Valid commit. (This patch is a bugfix, even.) > > > > 41ac2076a7c6 UefiCpuPkg CpuCommonFeaturesLib: Remove CPU generation check > > This is an invalid commit during the soft feature freeze. The change is > *not* a bugfix (it is a change that helps with the future use of a > function). And sufficient review was given only *after* the deadline, at > 13:10 UTC: > > > 734D49CCEBEEF84792F5B80ED585239D5C1476E8@SHSMSX104.ccr.corp.intel.com">http://mid.mail-archive.com/734D49CCEBEEF84792F5B80ED585239D5C1476E8@SHSMSX104.ccr.corp.intel.com >
I don't see this is in edk2 feature planning. We may need to give some guideline to define what change is a feature or a bug. > > > 59f20e8d7100 ShellPkg: Update DSC to use NetworkPkg's include fragment file > > Invalid commit. This is not a bugfix but a feature patch. (The commit > message should have referenced > <https://bugzilla.tianocore.org/show_bug.cgi?id=1293>.) The one R-b that > it received was posted *way* after the deadline, on 2019-05-20, at 00:28 > UTC: > > > 3CE959C139B4C44DBEA1810E3AA6F9000B7D3DB1@SHSMSX101.ccr.corp.intel.com">http://mid.mail-archive.com/3CE959C139B4C44DBEA1810E3AA6F9000B7D3DB1@SHSMSX101.ccr.corp.intel.com > > > > 48f43c2c56ee EmbeddedPkg: Update DSC to use NetworkPkg's include fragment > > file > > Invalid commit. This is a feature patch (and it should have referenced > <https://bugzilla.tianocore.org/show_bug.cgi?id=1293>). The Reviewed-by > was posted after the deadline, at 09:00 UTC. > > > 20190517090006.ko36fbne4vprai3w@bivouac.eciton.net">http://mid.mail-archive.com/20190517090006.ko36fbne4vprai3w@bivouac.eciton.net > The changes in ShellPkg and EmbeddedPkg refers to NetworkPkg's include fragment file. NetworkPkg's include fragment file have been added. Package DSC should use them. So, I regard those missing change as the bug fix. > > > 7b84de939489 ShellPkg: Display VENDOR_ID in ASCII when parsing PPTT > > Valid commit; the sufficient R-b was given in time: > > > 3CE959C139B4C44DBEA1810E3AA6F9000B7D21C2@SHSMSX101.ccr.corp.intel.com">http://mid.mail-archive.com/3CE959C139B4C44DBEA1810E3AA6F9000B7D21C2@SHSMSX101.ccr.corp.intel.com > > (This patch could also be considered a bugfix, I believe.) > > > > 911efe279ec3 ShellPkg: Add NetworkPkg/NetworkPkg.dec as the package > > dependency > > This is an invalid commit, during the soft feature freeze. Not only was > the reviewer feedback posted after the soft feature freeze, the patch > *itself* was posted after the same, on 2019-05-20, at 13:09. > > 20190520130920.9464-2-liming.gao@intel.com">http://mid.mail-archive.com/20190520130920.9464-2-liming.gao@intel.com > > And, this is not a bugfix but a feature enablement patch -- the commit > message says, "NetLib *will be moved* from MdeModulePkg and NetworkPkg" > (emphasis mine). > > The commit message also doesn't reference > <https://bugzilla.tianocore.org/show_bug.cgi?id=1293>. > > > > 110d4729b58e EmulatorPkg: Add NetworkPkg/NetworkPkg.dec as the package > > dependency > > Invalid, same as commit 911efe279ec3 above. > Network module movement patch have been reviewed. https://edk2.groups.io/g/devel/message/40840?p=,,,20,0,0,0::Created,,siyuan,20,2,0,31628785 When I plan to push it, I find the above issues. Because the feature to move network modules from MdeModulePkg to NetworkPkg have been reviewed, I regard them also bugs. Here, I don't want to argue whether they are feature or bug. I just want to share my thinking, and collect feedback, then work out the clear rule so that all developers can follow. > ... No, wait, this is worse: this patch had not received *any* review > feedback on the list: > > 20190520130920.9464-3-liming.gao@intel.com">http://mid.mail-archive.com/20190520130920.9464-3-liming.gao@intel.com > > but it was committed with Ray's R-b. I don't understand. > I see Ray has replied the mail. > > > cc99ea9422be Maintainers.txt: remove UTF-8 BOM wrongly added in commit > > 147e6e70 > > Valid commit: obvious bugfix. > > > > 66b845ae06f1 BaseTools: Fix private includes for FILE_GUID override > > Seems like a valid commit: a bugfix. > > There is one wart though -- the commit includes Bob's R-b, but that R-b > had never been posted to the list: > > 20190516111728.33584-1-bob.c.feng@intel.com">http://mid.mail-archive.com/20190516111728.33584-1-bob.c.feng@intel.com > > > > a7ef158b0752 BaseTools: Library hashing fix and optimization for --hash > > feature > > Looks like a bugfix to me, for a previously added feature (--hash), > hence arguably a valid commit. > > > So that's 5 invalid commits out of 11 commits, during the soft feature > freeze thus far. It's obvious that the current soft feature freeze > criteria are not being honored. So, again, either we need to enforce > them with tools better than just self-discipline, or we must change the > SFF criteria. 4 commits are all from mine. So, most people follows the rule. This is not bad. > > Thanks > Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#41230): https://edk2.groups.io/g/devel/message/41230 Mute This Topic: https://groups.io/mt/31653140/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-