Re: [PATCH] Features/Intel/Readme.md: Document meaning of "Complete" for features

["Oram, Isaac W" <isaac.w.oram@intel.com>]

Ray,

I don't think that this is a desirable rule.

I want to create feature packages that bundle frequently used together existing capabilities.  See the NetworkFeaturePkg for an example.  I also want to make feature packages for the USB stack, debug capabilities, and the like that are often aggregations of existing modules.

The Minimum Platform Architecture spec targets advanced features that are easy to enable for relatively inexperienced developers.  One way of doing that is to leverage the UEFI PI arch and its binary component support features.  The Minimum Platform Architecture aims to use this to enable a use case leveraging Firmware Volumes that looks like:
1:  Build NetworkFeaturePkg (this produces an FV, customized via PCD and/or static libraries as needed)
2:  Load FV (from shell, by injecting into an existing image using FMMT, Fiano, etc)
3:  Use network features and functionality

The model where the only way people extend a UEFI firmware image is by rebuilding a complete solution needs to end.  It is a misuse of the architecture in my estimation.  We have not had much success with fine granularity component binary use, i.e. individual PEIM and drivers.  Perhaps there is too much expertise needed.  Minimum Platform Architecture and Advanced Features aim to improve this by enabling larger granularity binary components that require less UEFI knowledge to use effectively.

I recognize that there is a competing vision that wants to make many small feature packages that are easy to build in or out based on simple PCD feature flags.  As that may improve developer's experience, it is not something I am strongly contesting.  However, I just don't see it as any different than MdeModulePkg.  It is the same strategy, just using packages to organize instead of directories.

The other consideration should include that we have a lot of existing users.  I don't want to move existing code around to make usable features.  If we move existing code to create the feature in the first place, we affect all the existing users, often for no immediate benefit.  If features become successful and widely used, then is a good time to refactor the code.  The difference is that at that time, the change is essentially behind an abstraction and so the change doesn't cause as much pointless work.  

Regards,
Isaac


-----Original Message-----
From: Ray Ni <niruiyu@users.noreply.github.com> 
Sent: Thursday, March 12, 2020 5:41 AM
To: devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@intel.com>; Dong, Eric <eric.dong@intel.com>; Chan, Amy <amy.chan@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>
Subject: [PATCH] Features/Intel/Readme.md: Document meaning of "Complete" for features

Today's document doesn't forbidden creation of a feature package with only interfaces and no code to implement the interfaces. Such feature package is useless.

Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Amy Chan <amy.chan@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
Cc: Isaac W Oram <isaac.w.oram@intel.com>
---
 Features/Intel/Readme.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Features/Intel/Readme.md b/Features/Intel/Readme.md index 9729f90a41..f0923e3d56 100644
--- a/Features/Intel/Readme.md
+++ b/Features/Intel/Readme.md
@@ -18,7 +18,7 @@ document as needed.
 Advanced features should be:
 * _Cohesive_, the feature should not contain any functionality unrelated to the feature.
 * _Complete_, the feature must have a complete design that minimizes dependencies. A feature package cannot directly
-  depend on another feature package.
+  depend on another feature package. A feature package must contain module(s) to implement the feature interfaces.
 * _Easy to Integrate_, the feature should expose well-defined software interfaces to use and configure the feature.
   * It should also present a set of simple and well-documented standard EDK II configuration options such as PCDs to
   configure the feature.
--
2.21.0.windows.1