Re: [edk2-devel] [edk2] Soft Feature Freeze starts today for edk2-stable201905

["Laszlo Ersek" <lersek@redhat.com>]

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:

  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:

  http://mid.mail-archive.com/DB6PR0802MB237582494362A29FEBE0DD3E84330@DB6PR0802MB2375.eurprd08.prod.outlook.com
  http://mid.mail-archive.com/3C0D5C461C9E904E8F62152F6274C0BB40BD4110@SHSMSX104.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:

  http://mid.mail-archive.com/734D49CCEBEEF84792F5B80ED585239D5C1476E8@SHSMSX104.ccr.corp.intel.com


> 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:

  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.

  http://mid.mail-archive.com/20190517090006.ko36fbne4vprai3w@bivouac.eciton.net


> 7b84de939489 ShellPkg: Display VENDOR_ID in ASCII when parsing PPTT

Valid commit; the sufficient R-b was given in time:

  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.

  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.

... No, wait, this is worse: this patch had not received *any* review
feedback on the list:

  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.


> 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:

  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.

Thanks
Laszlo