* [PATCH v1 0/1] Maintainers.txt: Review ownership for MdeModulePkg
@ 2019-07-17 1:44 Wu, Hao A
2019-07-17 1:44 ` [PATCH v1 1/1] Maintainers.txt: Fine-grained review " Wu, Hao A
0 siblings, 1 reply; 7+ messages in thread
From: Wu, Hao A @ 2019-07-17 1:44 UTC (permalink / raw)
To: devel
Cc: Hao A Wu, Andrew Fish, Laszlo Ersek, Leif Lindholm,
Michael D Kinney, Dandan Bi, Eric Dong, Liming Gao, Ray Ni,
Star Zeng, Zhichao Gao
This patch is also available at:
https://github.com/hwu25/edk2/tree/mdemodulepkg_reviewers
Also, please note that on the above branch, the proposed patch
(MdeModulePkg reviewers) is inserted between:
* [edk2-devel] [PATCH 4/4] Maintainers.txt: split out section "OvmfPkg: CSM modules"
(from Laszlo)
https://edk2.groups.io/g/devel/message/43732
and
* [edk2-devel] [PATCH 3/3] BaseTools: add GetMaintainer.py script
(from Leif)
https://edk2.groups.io/g/devel/message/43666
Cc: Andrew Fish <afish@apple.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Hao A Wu (1):
Maintainers.txt: Fine-grained review ownership for MdeModulePkg
Maintainers.txt | 151 +++++++++++++++++++-
1 file changed, 149 insertions(+), 2 deletions(-)
--
2.12.0.windows.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v1 1/1] Maintainers.txt: Fine-grained review ownership for MdeModulePkg
2019-07-17 1:44 [PATCH v1 0/1] Maintainers.txt: Review ownership for MdeModulePkg Wu, Hao A
@ 2019-07-17 1:44 ` Wu, Hao A
2019-07-17 12:41 ` Laszlo Ersek
0 siblings, 1 reply; 7+ messages in thread
From: Wu, Hao A @ 2019-07-17 1:44 UTC (permalink / raw)
To: devel
Cc: Hao A Wu, Andrew Fish, Laszlo Ersek, Leif Lindholm,
Michael D Kinney, Dandan Bi, Eric Dong, Liming Gao, Ray Ni,
Star Zeng, Zhichao Gao
This commit add the reviewers information for modules within MdeModulePkg.
For now the modules list includes:
ACPI
BDS
Console and Graphic
Core services (PEI, DXE and Runtime)
Device and Peripheral
Firmware Update
HII and UI
Reset
S3
SMBIOS
SMM
Variable
Please note that, for MdeModulePkg components not included in the above
list, the reviewers will fall back to the package maintainers.
Cc: Andrew Fish <afish@apple.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
---
Maintainers.txt | 151 +++++++++++++++++++-
1 file changed, 149 insertions(+), 2 deletions(-)
diff --git a/Maintainers.txt b/Maintainers.txt
index 18fd2ef43c..c1ae02045e 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -186,11 +186,158 @@ F: MdeModulePkg/
W: https://github.com/tianocore/tianocore.github.io/wiki/MdeModulePkg
M: Jian J Wang <jian.j.wang@intel.com>
M: Hao A Wu <hao.a.wu@intel.com>
+
+MdeModulePkg: ACPI modules
+F: MdeModulePkg/Universal/Acpi/
+F: MdeModulePkg/Include/*Acpi*.h
+R: Dandan Bi <dandan.bi@intel.com>
+R: Liming Gao <liming.gao@intel.com>
+
+MdeModulePkg: BDS modules
+F: MdeModulePkg/*BootManager*/
+F: MdeModulePkg/Universal/BdsDxe/
+F: MdeModulePkg/Universal/LoadFileOnFv2/
+F: MdeModulePkg/Universal/SecurityStubDxe/Defer3rdPartyImageLoad.*
+F: MdeModulePkg/Universal/DevicePathDxe/
+F: MdeModulePkg/Universal/DriverHealthManagerDxe/
+F: MdeModulePkg/Include/Library/UefiBootManagerLib.h
+R: Zhichao Gao <zhichao.gao@intel.com>
+R: Ray Ni <ray.ni@intel.com>
+
+MdeModulePkg: Console and Graphic modules
+F: MdeModulePkg/*Logo*/
+F: MdeModulePkg/Library/BaseBmpSupportLib/
+F: MdeModulePkg/Library/FrameBufferBltLib/
+F: MdeModulePkg/Universal/Console/
+F: MdeModulePkg/Include/*Logo*.h
+F: MdeModulePkg/Include/Guid/ConnectConInEvent.h
+F: MdeModulePkg/Include/Guid/Console*.h
+F: MdeModulePkg/Include/Guid/StandardErrorDevice.h
+F: MdeModulePkg/Include/Guid/TtyTerm.h
+F: MdeModulePkg/Include/Library/BmpSupportLib.h
+F: MdeModulePkg/Include/Library/FrameBufferBltLib.h
+R: Zhichao Gao <zhichao.gao@intel.com>
+R: Ray Ni <ray.ni@intel.com>
+
+MdeModulePkg: Core services (PEI, DXE and Runtime) modules
+F: MdeModulePkg/*Mem*/
+F: MdeModulePkg/*SectionExtract*/
+F: MdeModulePkg/*StatusCode*/
+F: MdeModulePkg/Application/DumpDynPcd/
+F: MdeModulePkg/Core/Dxe/
+F: MdeModulePkg/Core/DxeIplPeim/
+F: MdeModulePkg/Core/Pei/
+F: MdeModulePkg/Core/RuntimeDxe/
+F: MdeModulePkg/Library/*Decompress*/
+F: MdeModulePkg/Library/*Perf*/
+F: MdeModulePkg/Library/DxeSecurityManagementLib/
+F: MdeModulePkg/Universal/PCD/
+F: MdeModulePkg/Universal/PlatformDriOverrideDxe/
+F: MdeModulePkg/Universal/SecurityStubDxe/SecurityStub.c
+F: MdeModulePkg/Include/*Mem*.h
+F: MdeModulePkg/Include/*Pcd*.h
+F: MdeModulePkg/Include/*Perf*.h
+F: MdeModulePkg/Include/*StatusCode*.h
+F: MdeModulePkg/Include/Guid/Crc32GuidedSectionExtraction.h
+F: MdeModulePkg/Include/Guid/EventExitBootServiceFailed.h
+F: MdeModulePkg/Include/Guid/IdleLoopEvent.h
+F: MdeModulePkg/Include/Guid/LoadModuleAtFixedAddress.h
+F: MdeModulePkg/Include/Guid/LzmaDecompress.h
+F: MdeModulePkg/Include/Library/SecurityManagementLib.h
+R: Dandan Bi <dandan.bi@intel.com>
+R: Liming Gao <liming.gao@intel.com>
+
+MdeModulePkg: Device and Peripheral modules
+F: MdeModulePkg/*PciHostBridge*/
+F: MdeModulePkg/*Serial*/
+F: MdeModulePkg/Bus/
+F: MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/
+F: MdeModulePkg/Universal/Disk/
+F: MdeModulePkg/Universal/PcatSingleSegmentPciCfg2Pei/
+F: MdeModulePkg/Include/*Ata*.h
+F: MdeModulePkg/Include/*IoMmu*.h
+F: MdeModulePkg/Include/*NonDiscoverableDevice*.h
+F: MdeModulePkg/Include/*NvmExpress*.h
+F: MdeModulePkg/Include/*SerialPort*.h
+F: MdeModulePkg/Include/*SdMmc*.h
+F: MdeModulePkg/Include/*Ufs*.h
+F: MdeModulePkg/Include/*Usb*.h
+F: MdeModulePkg/Include/Guid/S3StorageDeviceInitList.h
+F: MdeModulePkg/Include/Guid/RecoveryDevice.h
+F: MdeModulePkg/Include/Library/PciHostBridgeLib.h
+F: MdeModulePkg/Include/Ppi/StorageSecurityCommand.h
+F: MdeModulePkg/Include/Protocol/Ps2Policy.h
+R: Hao A Wu <hao.a.wu@intel.com>
R: Ray Ni <ray.ni@intel.com>
- (especially for Bus, Universal/Console, Universal/Disk,
- Universal/BdsDxe and related libraries, header files)
+
+MdeModulePkg: Firmware Update modules
+F: MdeModulePkg/*Capsule*/
+F: MdeModulePkg/Library/DisplayUpdateProgressLib*/
+F: MdeModulePkg/Library/FmpAuthenticationLibNull/
+F: MdeModulePkg/Universal/Esrt*/
+F: MdeModulePkg/Include/*Capsule*.h
+F: MdeModulePkg/Include/Library/DisplayUpdateProgressLib.h
+F: MdeModulePkg/Include/Library/FmpAuthenticationLib.h
+F: MdeModulePkg/Include/Protocol/EsrtManagement.h
+F: MdeModulePkg/Include/Protocol/FirmwareManagementProgress.h
+R: Hao A Wu <hao.a.wu@intel.com>
+R: Liming Gao <liming.gao@intel.com>
+
+MdeModulePkg: HII and UI modules
+F: MdeModulePkg/*FileExplorer*/
+F: MdeModulePkg/*Hii*/
+F: MdeModulePkg/*Ui*/
+F: MdeModulePkg/Application/BootManagerMenuApp/
+F: MdeModulePkg/Library/CustomizedDisplayLib/
+F: MdeModulePkg/Universal/DisplayEngineDxe/
+F: MdeModulePkg/Universal/DriverSampleDxe/
+F: MdeModulePkg/Universal/SetupBrowserDxe/
+F: MdeModulePkg/Include/*FileExplorer*.h
+F: MdeModulePkg/Include/*FormBrowser*.h
+F: MdeModulePkg/Include/*Hii*.h
+F: MdeModulePkg/Include/Library/CustomizedDisplayLib.h
+F: MdeModulePkg/Include/Protocol/DisplayProtocol.h
+R: Dandan Bi <dandan.bi@intel.com>
+R: Eric Dong <eric.dong@intel.com>
+
+MdeModulePkg: Reset modules
+F: MdeModulePkg/*Reset*/
+F: MdeModulePkg/Include/*Reset*.h
+R: Zhichao Gao <zhichao.gao@intel.com>
+R: Ray Ni <ray.ni@intel.com>
+
+MdeModulePkg: S3 modules
+F: MdeModulePkg/*LockBox*/
+F: MdeModulePkg/Library/*S3*/
+F: MdeModulePkg/Include/*BootScript*.h
+F: MdeModulePkg/Include/*LockBox*.h
+F: MdeModulePkg/Include/*S3*.h
+R: Hao A Wu <hao.a.wu@intel.com>
+R: Eric Dong <eric.dong@intel.com>
+
+MdeModulePkg: SMBIOS modules
+F: MdeModulePkg/Universal/Smbios*/
+R: Dandan Bi <dandan.bi@intel.com>
R: Star Zeng <star.zeng@intel.com>
+MdeModulePkg: SMM modules
+F: MdeModulePkg/*Smi*/
+F: MdeModulePkg/*Smm*/
+F: MdeModulePkg/Include/*Smi*.h
+F: MdeModulePkg/Include/*Smm*.h
+R: Eric Dong <eric.dong@intel.com>
+R: Ray Ni <ray.ni@intel.com>
+
+MdeModulePkg: Variable modules
+F: MdeModulePkg/*Var*/
+F: MdeModulePkg/Universal/FaultTolerantWrite*/
+F: MdeModulePkg/Include/*/*FaultTolerantWrite*.h
+F: MdeModulePkg/Include/*/*Var*.h
+F: MdeModulePkg/Include/Guid/SystemNvDataGuid.h
+F: MdeModulePkg/Include/Protocol/SwapAddressRange.h
+R: Hao A Wu <hao.a.wu@intel.com>
+R: Liming Gao <liming.gao@intel.com>
+
MdePkg
F: MdePkg/
W: https://github.com/tianocore/tianocore.github.io/wiki/MdePkg
--
2.12.0.windows.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v1 1/1] Maintainers.txt: Fine-grained review ownership for MdeModulePkg
2019-07-17 1:44 ` [PATCH v1 1/1] Maintainers.txt: Fine-grained review " Wu, Hao A
@ 2019-07-17 12:41 ` Laszlo Ersek
2019-07-17 14:36 ` Leif Lindholm
2019-07-18 0:15 ` Wu, Hao A
0 siblings, 2 replies; 7+ messages in thread
From: Laszlo Ersek @ 2019-07-17 12:41 UTC (permalink / raw)
To: Hao A Wu, devel
Cc: Andrew Fish, Leif Lindholm, Michael D Kinney, Dandan Bi,
Eric Dong, Liming Gao, Ray Ni, Star Zeng, Zhichao Gao
On 07/17/19 03:44, Hao A Wu wrote:
> This commit add the reviewers information for modules within MdeModulePkg.
>
> For now the modules list includes:
> ACPI
> BDS
> Console and Graphic
> Core services (PEI, DXE and Runtime)
> Device and Peripheral
> Firmware Update
> HII and UI
> Reset
> S3
> SMBIOS
> SMM
> Variable
>
> Please note that, for MdeModulePkg components not included in the above
> list, the reviewers will fall back to the package maintainers.
>
> Cc: Andrew Fish <afish@apple.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Dandan Bi <dandan.bi@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
> ---
> Maintainers.txt | 151 +++++++++++++++++++-
> 1 file changed, 149 insertions(+), 2 deletions(-)
>
> diff --git a/Maintainers.txt b/Maintainers.txt
> index 18fd2ef43c..c1ae02045e 100644
> --- a/Maintainers.txt
> +++ b/Maintainers.txt
> @@ -186,11 +186,158 @@ F: MdeModulePkg/
> W: https://github.com/tianocore/tianocore.github.io/wiki/MdeModulePkg
> M: Jian J Wang <jian.j.wang@intel.com>
> M: Hao A Wu <hao.a.wu@intel.com>
> +
(1) First comment here. This is actually a comment on the patch
submission process, not the patch itself. I *suggest* (but do not
*require*) that such changes be formatted for submission with at least
-U6 (i.e. 6 lines of context, at the minimum). That makes the review
much easier.
Anyway, to explain the change to myself: basically this cuts the
existent MdeModulePkg section in half, keeping Jian and Hao (both "M")
assigned at the top-level, and pushing down all other folks ("R"s: Ray
and Star) to other subsystems.
That's fine.
> +MdeModulePkg: ACPI modules
> +F: MdeModulePkg/Universal/Acpi/
> +F: MdeModulePkg/Include/*Acpi*.h
> +R: Dandan Bi <dandan.bi@intel.com>
> +R: Liming Gao <liming.gao@intel.com>
Basic formatting (F followed by R) is fine.
(2) General request (applies to all sections added in this patch): I
suggest to sort the "F" patterns lexicographically. That will make
future changes to the F patterns more localized and cleaner.
> +
> +MdeModulePkg: BDS modules
> +F: MdeModulePkg/*BootManager*/
> +F: MdeModulePkg/Universal/BdsDxe/
> +F: MdeModulePkg/Universal/LoadFileOnFv2/
> +F: MdeModulePkg/Universal/SecurityStubDxe/Defer3rdPartyImageLoad.*
> +F: MdeModulePkg/Universal/DevicePathDxe/
> +F: MdeModulePkg/Universal/DriverHealthManagerDxe/
> +F: MdeModulePkg/Include/Library/UefiBootManagerLib.h
> +R: Zhichao Gao <zhichao.gao@intel.com>
> +R: Ray Ni <ray.ni@intel.com>
> +
> +MdeModulePkg: Console and Graphic modules
(3) Suggesting a typo fix: "Graphic*s*" modules. "Graphic modules"
suggests they are modules restricted from an under-aged audience. :)
(I'm not a native English speaker though.)
> +F: MdeModulePkg/*Logo*/
> +F: MdeModulePkg/Library/BaseBmpSupportLib/
> +F: MdeModulePkg/Library/FrameBufferBltLib/
> +F: MdeModulePkg/Universal/Console/
> +F: MdeModulePkg/Include/*Logo*.h
> +F: MdeModulePkg/Include/Guid/ConnectConInEvent.h
> +F: MdeModulePkg/Include/Guid/Console*.h
> +F: MdeModulePkg/Include/Guid/StandardErrorDevice.h
> +F: MdeModulePkg/Include/Guid/TtyTerm.h
> +F: MdeModulePkg/Include/Library/BmpSupportLib.h
> +F: MdeModulePkg/Include/Library/FrameBufferBltLib.h
> +R: Zhichao Gao <zhichao.gao@intel.com>
> +R: Ray Ni <ray.ni@intel.com>
> +
> +MdeModulePkg: Core services (PEI, DXE and Runtime) modules
> +F: MdeModulePkg/*Mem*/
> +F: MdeModulePkg/*SectionExtract*/
> +F: MdeModulePkg/*StatusCode*/
> +F: MdeModulePkg/Application/DumpDynPcd/
> +F: MdeModulePkg/Core/Dxe/
> +F: MdeModulePkg/Core/DxeIplPeim/
> +F: MdeModulePkg/Core/Pei/
> +F: MdeModulePkg/Core/RuntimeDxe/
> +F: MdeModulePkg/Library/*Decompress*/
> +F: MdeModulePkg/Library/*Perf*/
> +F: MdeModulePkg/Library/DxeSecurityManagementLib/
> +F: MdeModulePkg/Universal/PCD/
> +F: MdeModulePkg/Universal/PlatformDriOverrideDxe/
> +F: MdeModulePkg/Universal/SecurityStubDxe/SecurityStub.c
> +F: MdeModulePkg/Include/*Mem*.h
> +F: MdeModulePkg/Include/*Pcd*.h
> +F: MdeModulePkg/Include/*Perf*.h
> +F: MdeModulePkg/Include/*StatusCode*.h
> +F: MdeModulePkg/Include/Guid/Crc32GuidedSectionExtraction.h
> +F: MdeModulePkg/Include/Guid/EventExitBootServiceFailed.h
> +F: MdeModulePkg/Include/Guid/IdleLoopEvent.h
> +F: MdeModulePkg/Include/Guid/LoadModuleAtFixedAddress.h
> +F: MdeModulePkg/Include/Guid/LzmaDecompress.h
> +F: MdeModulePkg/Include/Library/SecurityManagementLib.h
> +R: Dandan Bi <dandan.bi@intel.com>
> +R: Liming Gao <liming.gao@intel.com>
> +
> +MdeModulePkg: Device and Peripheral modules
> +F: MdeModulePkg/*PciHostBridge*/
> +F: MdeModulePkg/*Serial*/
> +F: MdeModulePkg/Bus/
> +F: MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/
> +F: MdeModulePkg/Universal/Disk/
> +F: MdeModulePkg/Universal/PcatSingleSegmentPciCfg2Pei/
> +F: MdeModulePkg/Include/*Ata*.h
> +F: MdeModulePkg/Include/*IoMmu*.h
> +F: MdeModulePkg/Include/*NonDiscoverableDevice*.h
> +F: MdeModulePkg/Include/*NvmExpress*.h
> +F: MdeModulePkg/Include/*SerialPort*.h
> +F: MdeModulePkg/Include/*SdMmc*.h
> +F: MdeModulePkg/Include/*Ufs*.h
> +F: MdeModulePkg/Include/*Usb*.h
> +F: MdeModulePkg/Include/Guid/S3StorageDeviceInitList.h
> +F: MdeModulePkg/Include/Guid/RecoveryDevice.h
> +F: MdeModulePkg/Include/Library/PciHostBridgeLib.h
> +F: MdeModulePkg/Include/Ppi/StorageSecurityCommand.h
> +F: MdeModulePkg/Include/Protocol/Ps2Policy.h
> +R: Hao A Wu <hao.a.wu@intel.com>
> R: Ray Ni <ray.ni@intel.com>
> - (especially for Bus, Universal/Console, Universal/Disk,
> - Universal/BdsDxe and related libraries, header files)
> +
> +MdeModulePkg: Firmware Update modules
> +F: MdeModulePkg/*Capsule*/
> +F: MdeModulePkg/Library/DisplayUpdateProgressLib*/
> +F: MdeModulePkg/Library/FmpAuthenticationLibNull/
> +F: MdeModulePkg/Universal/Esrt*/
> +F: MdeModulePkg/Include/*Capsule*.h
> +F: MdeModulePkg/Include/Library/DisplayUpdateProgressLib.h
> +F: MdeModulePkg/Include/Library/FmpAuthenticationLib.h
> +F: MdeModulePkg/Include/Protocol/EsrtManagement.h
> +F: MdeModulePkg/Include/Protocol/FirmwareManagementProgress.h
> +R: Hao A Wu <hao.a.wu@intel.com>
> +R: Liming Gao <liming.gao@intel.com>
> +
> +MdeModulePkg: HII and UI modules
> +F: MdeModulePkg/*FileExplorer*/
> +F: MdeModulePkg/*Hii*/
> +F: MdeModulePkg/*Ui*/
> +F: MdeModulePkg/Application/BootManagerMenuApp/
> +F: MdeModulePkg/Library/CustomizedDisplayLib/
> +F: MdeModulePkg/Universal/DisplayEngineDxe/
> +F: MdeModulePkg/Universal/DriverSampleDxe/
> +F: MdeModulePkg/Universal/SetupBrowserDxe/
> +F: MdeModulePkg/Include/*FileExplorer*.h
> +F: MdeModulePkg/Include/*FormBrowser*.h
> +F: MdeModulePkg/Include/*Hii*.h
> +F: MdeModulePkg/Include/Library/CustomizedDisplayLib.h
> +F: MdeModulePkg/Include/Protocol/DisplayProtocol.h
> +R: Dandan Bi <dandan.bi@intel.com>
> +R: Eric Dong <eric.dong@intel.com>
> +
> +MdeModulePkg: Reset modules
> +F: MdeModulePkg/*Reset*/
> +F: MdeModulePkg/Include/*Reset*.h
> +R: Zhichao Gao <zhichao.gao@intel.com>
> +R: Ray Ni <ray.ni@intel.com>
> +
> +MdeModulePkg: S3 modules
(4) Can we say "ACPI S3 modules"? (Not insisting, just suggesting.)
> +F: MdeModulePkg/*LockBox*/
> +F: MdeModulePkg/Library/*S3*/
> +F: MdeModulePkg/Include/*BootScript*.h
> +F: MdeModulePkg/Include/*LockBox*.h
> +F: MdeModulePkg/Include/*S3*.h
> +R: Hao A Wu <hao.a.wu@intel.com>
> +R: Eric Dong <eric.dong@intel.com>
> +
> +MdeModulePkg: SMBIOS modules
> +F: MdeModulePkg/Universal/Smbios*/
> +R: Dandan Bi <dandan.bi@intel.com>
> R: Star Zeng <star.zeng@intel.com>
>
> +MdeModulePkg: SMM modules
(5) Can we use the more generic term:
Management Mode (MM, SMM) modules
?
> +F: MdeModulePkg/*Smi*/
> +F: MdeModulePkg/*Smm*/
> +F: MdeModulePkg/Include/*Smi*.h
> +F: MdeModulePkg/Include/*Smm*.h
> +R: Eric Dong <eric.dong@intel.com>
> +R: Ray Ni <ray.ni@intel.com>
> +
> +MdeModulePkg: Variable modules
(6) Can we say: "UEFI Variable modules"?
> +F: MdeModulePkg/*Var*/
> +F: MdeModulePkg/Universal/FaultTolerantWrite*/
> +F: MdeModulePkg/Include/*/*FaultTolerantWrite*.h
> +F: MdeModulePkg/Include/*/*Var*.h
> +F: MdeModulePkg/Include/Guid/SystemNvDataGuid.h
> +F: MdeModulePkg/Include/Protocol/SwapAddressRange.h
> +R: Hao A Wu <hao.a.wu@intel.com>
> +R: Liming Gao <liming.gao@intel.com>
> +
(7) Finally, a high level comment: personally, if I were listed as one
of the Reviewers, I'd prefer one patch per section added. But, that
preference is up to the Reviewers, of course.
I guess the only point I'd really like to see fixed is (2) -- the
sorting of "F" patterns. The rest is optional, from my side.
Thanks!
Laszlo
> MdePkg
> F: MdePkg/
> W: https://github.com/tianocore/tianocore.github.io/wiki/MdePkg
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1 1/1] Maintainers.txt: Fine-grained review ownership for MdeModulePkg
2019-07-17 12:41 ` Laszlo Ersek
@ 2019-07-17 14:36 ` Leif Lindholm
2019-07-17 15:19 ` Ni, Ray
2019-07-18 5:50 ` [edk2-devel] " Wu, Hao A
2019-07-18 0:15 ` Wu, Hao A
1 sibling, 2 replies; 7+ messages in thread
From: Leif Lindholm @ 2019-07-17 14:36 UTC (permalink / raw)
To: Laszlo Ersek
Cc: Hao A Wu, devel, Andrew Fish, Michael D Kinney, Dandan Bi,
Eric Dong, Liming Gao, Ray Ni, Star Zeng, Zhichao Gao
Top-posting because lazy.
Basically, I agree with all of Laszlo's comments.
I would be happy for this to remain a single patch, however. Partly
because I'm lazy. But also because this is effectively "converting"
a single package (MdeModulePkg) into multiple sections. (This is not a
strong preference.)
I would however prefer a v2 with sorted F:-patterns.
And of course, Acks/Reviews from everyone affected.
Best Regards,
Leif
On Wed, Jul 17, 2019 at 02:41:15PM +0200, Laszlo Ersek wrote:
> On 07/17/19 03:44, Hao A Wu wrote:
> > This commit add the reviewers information for modules within MdeModulePkg.
> >
> > For now the modules list includes:
> > ACPI
> > BDS
> > Console and Graphic
> > Core services (PEI, DXE and Runtime)
> > Device and Peripheral
> > Firmware Update
> > HII and UI
> > Reset
> > S3
> > SMBIOS
> > SMM
> > Variable
> >
> > Please note that, for MdeModulePkg components not included in the above
> > list, the reviewers will fall back to the package maintainers.
> >
> > Cc: Andrew Fish <afish@apple.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Leif Lindholm <leif.lindholm@linaro.org>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Dandan Bi <dandan.bi@intel.com>
> > Cc: Eric Dong <eric.dong@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> > Cc: Zhichao Gao <zhichao.gao@intel.com>
> > Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
> > ---
> > Maintainers.txt | 151 +++++++++++++++++++-
> > 1 file changed, 149 insertions(+), 2 deletions(-)
> >
> > diff --git a/Maintainers.txt b/Maintainers.txt
> > index 18fd2ef43c..c1ae02045e 100644
> > --- a/Maintainers.txt
> > +++ b/Maintainers.txt
> > @@ -186,11 +186,158 @@ F: MdeModulePkg/
> > W: https://github.com/tianocore/tianocore.github.io/wiki/MdeModulePkg
> > M: Jian J Wang <jian.j.wang@intel.com>
> > M: Hao A Wu <hao.a.wu@intel.com>
> > +
>
> (1) First comment here. This is actually a comment on the patch
> submission process, not the patch itself. I *suggest* (but do not
> *require*) that such changes be formatted for submission with at least
> -U6 (i.e. 6 lines of context, at the minimum). That makes the review
> much easier.
>
> Anyway, to explain the change to myself: basically this cuts the
> existent MdeModulePkg section in half, keeping Jian and Hao (both "M")
> assigned at the top-level, and pushing down all other folks ("R"s: Ray
> and Star) to other subsystems.
>
> That's fine.
>
>
> > +MdeModulePkg: ACPI modules
> > +F: MdeModulePkg/Universal/Acpi/
> > +F: MdeModulePkg/Include/*Acpi*.h
> > +R: Dandan Bi <dandan.bi@intel.com>
> > +R: Liming Gao <liming.gao@intel.com>
>
> Basic formatting (F followed by R) is fine.
>
> (2) General request (applies to all sections added in this patch): I
> suggest to sort the "F" patterns lexicographically. That will make
> future changes to the F patterns more localized and cleaner.
>
> > +
> > +MdeModulePkg: BDS modules
> > +F: MdeModulePkg/*BootManager*/
> > +F: MdeModulePkg/Universal/BdsDxe/
> > +F: MdeModulePkg/Universal/LoadFileOnFv2/
> > +F: MdeModulePkg/Universal/SecurityStubDxe/Defer3rdPartyImageLoad.*
> > +F: MdeModulePkg/Universal/DevicePathDxe/
> > +F: MdeModulePkg/Universal/DriverHealthManagerDxe/
> > +F: MdeModulePkg/Include/Library/UefiBootManagerLib.h
> > +R: Zhichao Gao <zhichao.gao@intel.com>
> > +R: Ray Ni <ray.ni@intel.com>
> > +
> > +MdeModulePkg: Console and Graphic modules
>
> (3) Suggesting a typo fix: "Graphic*s*" modules. "Graphic modules"
> suggests they are modules restricted from an under-aged audience. :)
>
> (I'm not a native English speaker though.)
>
>
> > +F: MdeModulePkg/*Logo*/
> > +F: MdeModulePkg/Library/BaseBmpSupportLib/
> > +F: MdeModulePkg/Library/FrameBufferBltLib/
> > +F: MdeModulePkg/Universal/Console/
> > +F: MdeModulePkg/Include/*Logo*.h
> > +F: MdeModulePkg/Include/Guid/ConnectConInEvent.h
> > +F: MdeModulePkg/Include/Guid/Console*.h
> > +F: MdeModulePkg/Include/Guid/StandardErrorDevice.h
> > +F: MdeModulePkg/Include/Guid/TtyTerm.h
> > +F: MdeModulePkg/Include/Library/BmpSupportLib.h
> > +F: MdeModulePkg/Include/Library/FrameBufferBltLib.h
> > +R: Zhichao Gao <zhichao.gao@intel.com>
> > +R: Ray Ni <ray.ni@intel.com>
> > +
> > +MdeModulePkg: Core services (PEI, DXE and Runtime) modules
> > +F: MdeModulePkg/*Mem*/
> > +F: MdeModulePkg/*SectionExtract*/
> > +F: MdeModulePkg/*StatusCode*/
> > +F: MdeModulePkg/Application/DumpDynPcd/
> > +F: MdeModulePkg/Core/Dxe/
> > +F: MdeModulePkg/Core/DxeIplPeim/
> > +F: MdeModulePkg/Core/Pei/
> > +F: MdeModulePkg/Core/RuntimeDxe/
> > +F: MdeModulePkg/Library/*Decompress*/
> > +F: MdeModulePkg/Library/*Perf*/
> > +F: MdeModulePkg/Library/DxeSecurityManagementLib/
> > +F: MdeModulePkg/Universal/PCD/
> > +F: MdeModulePkg/Universal/PlatformDriOverrideDxe/
> > +F: MdeModulePkg/Universal/SecurityStubDxe/SecurityStub.c
> > +F: MdeModulePkg/Include/*Mem*.h
> > +F: MdeModulePkg/Include/*Pcd*.h
> > +F: MdeModulePkg/Include/*Perf*.h
> > +F: MdeModulePkg/Include/*StatusCode*.h
> > +F: MdeModulePkg/Include/Guid/Crc32GuidedSectionExtraction.h
> > +F: MdeModulePkg/Include/Guid/EventExitBootServiceFailed.h
> > +F: MdeModulePkg/Include/Guid/IdleLoopEvent.h
> > +F: MdeModulePkg/Include/Guid/LoadModuleAtFixedAddress.h
> > +F: MdeModulePkg/Include/Guid/LzmaDecompress.h
> > +F: MdeModulePkg/Include/Library/SecurityManagementLib.h
> > +R: Dandan Bi <dandan.bi@intel.com>
> > +R: Liming Gao <liming.gao@intel.com>
> > +
> > +MdeModulePkg: Device and Peripheral modules
> > +F: MdeModulePkg/*PciHostBridge*/
> > +F: MdeModulePkg/*Serial*/
> > +F: MdeModulePkg/Bus/
> > +F: MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/
> > +F: MdeModulePkg/Universal/Disk/
> > +F: MdeModulePkg/Universal/PcatSingleSegmentPciCfg2Pei/
> > +F: MdeModulePkg/Include/*Ata*.h
> > +F: MdeModulePkg/Include/*IoMmu*.h
> > +F: MdeModulePkg/Include/*NonDiscoverableDevice*.h
> > +F: MdeModulePkg/Include/*NvmExpress*.h
> > +F: MdeModulePkg/Include/*SerialPort*.h
> > +F: MdeModulePkg/Include/*SdMmc*.h
> > +F: MdeModulePkg/Include/*Ufs*.h
> > +F: MdeModulePkg/Include/*Usb*.h
> > +F: MdeModulePkg/Include/Guid/S3StorageDeviceInitList.h
> > +F: MdeModulePkg/Include/Guid/RecoveryDevice.h
> > +F: MdeModulePkg/Include/Library/PciHostBridgeLib.h
> > +F: MdeModulePkg/Include/Ppi/StorageSecurityCommand.h
> > +F: MdeModulePkg/Include/Protocol/Ps2Policy.h
> > +R: Hao A Wu <hao.a.wu@intel.com>
> > R: Ray Ni <ray.ni@intel.com>
> > - (especially for Bus, Universal/Console, Universal/Disk,
> > - Universal/BdsDxe and related libraries, header files)
> > +
> > +MdeModulePkg: Firmware Update modules
> > +F: MdeModulePkg/*Capsule*/
> > +F: MdeModulePkg/Library/DisplayUpdateProgressLib*/
> > +F: MdeModulePkg/Library/FmpAuthenticationLibNull/
> > +F: MdeModulePkg/Universal/Esrt*/
> > +F: MdeModulePkg/Include/*Capsule*.h
> > +F: MdeModulePkg/Include/Library/DisplayUpdateProgressLib.h
> > +F: MdeModulePkg/Include/Library/FmpAuthenticationLib.h
> > +F: MdeModulePkg/Include/Protocol/EsrtManagement.h
> > +F: MdeModulePkg/Include/Protocol/FirmwareManagementProgress.h
> > +R: Hao A Wu <hao.a.wu@intel.com>
> > +R: Liming Gao <liming.gao@intel.com>
> > +
> > +MdeModulePkg: HII and UI modules
> > +F: MdeModulePkg/*FileExplorer*/
> > +F: MdeModulePkg/*Hii*/
> > +F: MdeModulePkg/*Ui*/
> > +F: MdeModulePkg/Application/BootManagerMenuApp/
> > +F: MdeModulePkg/Library/CustomizedDisplayLib/
> > +F: MdeModulePkg/Universal/DisplayEngineDxe/
> > +F: MdeModulePkg/Universal/DriverSampleDxe/
> > +F: MdeModulePkg/Universal/SetupBrowserDxe/
> > +F: MdeModulePkg/Include/*FileExplorer*.h
> > +F: MdeModulePkg/Include/*FormBrowser*.h
> > +F: MdeModulePkg/Include/*Hii*.h
> > +F: MdeModulePkg/Include/Library/CustomizedDisplayLib.h
> > +F: MdeModulePkg/Include/Protocol/DisplayProtocol.h
> > +R: Dandan Bi <dandan.bi@intel.com>
> > +R: Eric Dong <eric.dong@intel.com>
> > +
> > +MdeModulePkg: Reset modules
> > +F: MdeModulePkg/*Reset*/
> > +F: MdeModulePkg/Include/*Reset*.h
> > +R: Zhichao Gao <zhichao.gao@intel.com>
> > +R: Ray Ni <ray.ni@intel.com>
> > +
> > +MdeModulePkg: S3 modules
>
> (4) Can we say "ACPI S3 modules"? (Not insisting, just suggesting.)
>
> > +F: MdeModulePkg/*LockBox*/
> > +F: MdeModulePkg/Library/*S3*/
> > +F: MdeModulePkg/Include/*BootScript*.h
> > +F: MdeModulePkg/Include/*LockBox*.h
> > +F: MdeModulePkg/Include/*S3*.h
> > +R: Hao A Wu <hao.a.wu@intel.com>
> > +R: Eric Dong <eric.dong@intel.com>
> > +
> > +MdeModulePkg: SMBIOS modules
> > +F: MdeModulePkg/Universal/Smbios*/
> > +R: Dandan Bi <dandan.bi@intel.com>
> > R: Star Zeng <star.zeng@intel.com>
> >
> > +MdeModulePkg: SMM modules
>
> (5) Can we use the more generic term:
>
> Management Mode (MM, SMM) modules
>
> ?
>
> > +F: MdeModulePkg/*Smi*/
> > +F: MdeModulePkg/*Smm*/
> > +F: MdeModulePkg/Include/*Smi*.h
> > +F: MdeModulePkg/Include/*Smm*.h
> > +R: Eric Dong <eric.dong@intel.com>
> > +R: Ray Ni <ray.ni@intel.com>
> > +
> > +MdeModulePkg: Variable modules
>
> (6) Can we say: "UEFI Variable modules"?
>
> > +F: MdeModulePkg/*Var*/
> > +F: MdeModulePkg/Universal/FaultTolerantWrite*/
> > +F: MdeModulePkg/Include/*/*FaultTolerantWrite*.h
> > +F: MdeModulePkg/Include/*/*Var*.h
> > +F: MdeModulePkg/Include/Guid/SystemNvDataGuid.h
> > +F: MdeModulePkg/Include/Protocol/SwapAddressRange.h
> > +R: Hao A Wu <hao.a.wu@intel.com>
> > +R: Liming Gao <liming.gao@intel.com>
> > +
>
> (7) Finally, a high level comment: personally, if I were listed as one
> of the Reviewers, I'd prefer one patch per section added. But, that
> preference is up to the Reviewers, of course.
>
> I guess the only point I'd really like to see fixed is (2) -- the
> sorting of "F" patterns. The rest is optional, from my side.
>
> Thanks!
> Laszlo
>
>
> > MdePkg
> > F: MdePkg/
> > W: https://github.com/tianocore/tianocore.github.io/wiki/MdePkg
> >
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1 1/1] Maintainers.txt: Fine-grained review ownership for MdeModulePkg
2019-07-17 14:36 ` Leif Lindholm
@ 2019-07-17 15:19 ` Ni, Ray
2019-07-18 5:50 ` [edk2-devel] " Wu, Hao A
1 sibling, 0 replies; 7+ messages in thread
From: Ni, Ray @ 2019-07-17 15:19 UTC (permalink / raw)
To: Leif Lindholm, Laszlo Ersek
Cc: Wu, Hao A, devel@edk2.groups.io, Andrew Fish, Kinney, Michael D,
Bi, Dandan, Dong, Eric, Gao, Liming, Zeng, Star, Gao, Zhichao
Regarding the role assigned to me, Reviewed-by: Ray Ni <ray.ni@intel.com>
> -----Original Message-----
> From: Leif Lindholm <leif.lindholm@linaro.org>
> Sent: Wednesday, July 17, 2019 10:37 PM
> To: Laszlo Ersek <lersek@redhat.com>
> Cc: Wu, Hao A <hao.a.wu@intel.com>; devel@edk2.groups.io; Andrew Fish
> <afish@apple.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Bi,
> Dandan <dandan.bi@intel.com>; Dong, Eric <eric.dong@intel.com>; Gao,
> Liming <liming.gao@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng, Star
> <star.zeng@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>
> Subject: Re: [PATCH v1 1/1] Maintainers.txt: Fine-grained review ownership for
> MdeModulePkg
>
> Top-posting because lazy.
>
> Basically, I agree with all of Laszlo's comments.
> I would be happy for this to remain a single patch, however. Partly because I'm
> lazy. But also because this is effectively "converting"
> a single package (MdeModulePkg) into multiple sections. (This is not a strong
> preference.)
>
> I would however prefer a v2 with sorted F:-patterns.
>
> And of course, Acks/Reviews from everyone affected.
>
> Best Regards,
>
> Leif
>
> On Wed, Jul 17, 2019 at 02:41:15PM +0200, Laszlo Ersek wrote:
> > On 07/17/19 03:44, Hao A Wu wrote:
> > > This commit add the reviewers information for modules within
> MdeModulePkg.
> > >
> > > For now the modules list includes:
> > > ACPI
> > > BDS
> > > Console and Graphic
> > > Core services (PEI, DXE and Runtime) Device and Peripheral Firmware
> > > Update HII and UI Reset
> > > S3
> > > SMBIOS
> > > SMM
> > > Variable
> > >
> > > Please note that, for MdeModulePkg components not included in the
> > > above list, the reviewers will fall back to the package maintainers.
> > >
> > > Cc: Andrew Fish <afish@apple.com>
> > > Cc: Laszlo Ersek <lersek@redhat.com>
> > > Cc: Leif Lindholm <leif.lindholm@linaro.org>
> > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > Cc: Dandan Bi <dandan.bi@intel.com>
> > > Cc: Eric Dong <eric.dong@intel.com>
> > > Cc: Liming Gao <liming.gao@intel.com>
> > > Cc: Ray Ni <ray.ni@intel.com>
> > > Cc: Star Zeng <star.zeng@intel.com>
> > > Cc: Zhichao Gao <zhichao.gao@intel.com>
> > > Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
> > > ---
> > > Maintainers.txt | 151 +++++++++++++++++++-
> > > 1 file changed, 149 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Maintainers.txt b/Maintainers.txt index
> > > 18fd2ef43c..c1ae02045e 100644
> > > --- a/Maintainers.txt
> > > +++ b/Maintainers.txt
> > > @@ -186,11 +186,158 @@ F: MdeModulePkg/
> > > W:
> > > https://github.com/tianocore/tianocore.github.io/wiki/MdeModulePkg
> > > M: Jian J Wang <jian.j.wang@intel.com>
> > > M: Hao A Wu <hao.a.wu@intel.com>
> > > +
> >
> > (1) First comment here. This is actually a comment on the patch
> > submission process, not the patch itself. I *suggest* (but do not
> > *require*) that such changes be formatted for submission with at least
> > -U6 (i.e. 6 lines of context, at the minimum). That makes the review
> > much easier.
> >
> > Anyway, to explain the change to myself: basically this cuts the
> > existent MdeModulePkg section in half, keeping Jian and Hao (both "M")
> > assigned at the top-level, and pushing down all other folks ("R"s: Ray
> > and Star) to other subsystems.
> >
> > That's fine.
> >
> >
> > > +MdeModulePkg: ACPI modules
> > > +F: MdeModulePkg/Universal/Acpi/
> > > +F: MdeModulePkg/Include/*Acpi*.h
> > > +R: Dandan Bi <dandan.bi@intel.com>
> > > +R: Liming Gao <liming.gao@intel.com>
> >
> > Basic formatting (F followed by R) is fine.
> >
> > (2) General request (applies to all sections added in this patch): I
> > suggest to sort the "F" patterns lexicographically. That will make
> > future changes to the F patterns more localized and cleaner.
> >
> > > +
> > > +MdeModulePkg: BDS modules
> > > +F: MdeModulePkg/*BootManager*/
> > > +F: MdeModulePkg/Universal/BdsDxe/
> > > +F: MdeModulePkg/Universal/LoadFileOnFv2/
> > > +F: MdeModulePkg/Universal/SecurityStubDxe/Defer3rdPartyImageLoad.*
> > > +F: MdeModulePkg/Universal/DevicePathDxe/
> > > +F: MdeModulePkg/Universal/DriverHealthManagerDxe/
> > > +F: MdeModulePkg/Include/Library/UefiBootManagerLib.h
> > > +R: Zhichao Gao <zhichao.gao@intel.com>
> > > +R: Ray Ni <ray.ni@intel.com>
> > > +
> > > +MdeModulePkg: Console and Graphic modules
> >
> > (3) Suggesting a typo fix: "Graphic*s*" modules. "Graphic modules"
> > suggests they are modules restricted from an under-aged audience. :)
> >
> > (I'm not a native English speaker though.)
> >
> >
> > > +F: MdeModulePkg/*Logo*/
> > > +F: MdeModulePkg/Library/BaseBmpSupportLib/
> > > +F: MdeModulePkg/Library/FrameBufferBltLib/
> > > +F: MdeModulePkg/Universal/Console/
> > > +F: MdeModulePkg/Include/*Logo*.h
> > > +F: MdeModulePkg/Include/Guid/ConnectConInEvent.h
> > > +F: MdeModulePkg/Include/Guid/Console*.h
> > > +F: MdeModulePkg/Include/Guid/StandardErrorDevice.h
> > > +F: MdeModulePkg/Include/Guid/TtyTerm.h
> > > +F: MdeModulePkg/Include/Library/BmpSupportLib.h
> > > +F: MdeModulePkg/Include/Library/FrameBufferBltLib.h
> > > +R: Zhichao Gao <zhichao.gao@intel.com>
> > > +R: Ray Ni <ray.ni@intel.com>
> > > +
> > > +MdeModulePkg: Core services (PEI, DXE and Runtime) modules
> > > +F: MdeModulePkg/*Mem*/
> > > +F: MdeModulePkg/*SectionExtract*/
> > > +F: MdeModulePkg/*StatusCode*/
> > > +F: MdeModulePkg/Application/DumpDynPcd/
> > > +F: MdeModulePkg/Core/Dxe/
> > > +F: MdeModulePkg/Core/DxeIplPeim/
> > > +F: MdeModulePkg/Core/Pei/
> > > +F: MdeModulePkg/Core/RuntimeDxe/
> > > +F: MdeModulePkg/Library/*Decompress*/
> > > +F: MdeModulePkg/Library/*Perf*/
> > > +F: MdeModulePkg/Library/DxeSecurityManagementLib/
> > > +F: MdeModulePkg/Universal/PCD/
> > > +F: MdeModulePkg/Universal/PlatformDriOverrideDxe/
> > > +F: MdeModulePkg/Universal/SecurityStubDxe/SecurityStub.c
> > > +F: MdeModulePkg/Include/*Mem*.h
> > > +F: MdeModulePkg/Include/*Pcd*.h
> > > +F: MdeModulePkg/Include/*Perf*.h
> > > +F: MdeModulePkg/Include/*StatusCode*.h
> > > +F: MdeModulePkg/Include/Guid/Crc32GuidedSectionExtraction.h
> > > +F: MdeModulePkg/Include/Guid/EventExitBootServiceFailed.h
> > > +F: MdeModulePkg/Include/Guid/IdleLoopEvent.h
> > > +F: MdeModulePkg/Include/Guid/LoadModuleAtFixedAddress.h
> > > +F: MdeModulePkg/Include/Guid/LzmaDecompress.h
> > > +F: MdeModulePkg/Include/Library/SecurityManagementLib.h
> > > +R: Dandan Bi <dandan.bi@intel.com>
> > > +R: Liming Gao <liming.gao@intel.com>
> > > +
> > > +MdeModulePkg: Device and Peripheral modules
> > > +F: MdeModulePkg/*PciHostBridge*/
> > > +F: MdeModulePkg/*Serial*/
> > > +F: MdeModulePkg/Bus/
> > > +F: MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/
> > > +F: MdeModulePkg/Universal/Disk/
> > > +F: MdeModulePkg/Universal/PcatSingleSegmentPciCfg2Pei/
> > > +F: MdeModulePkg/Include/*Ata*.h
> > > +F: MdeModulePkg/Include/*IoMmu*.h
> > > +F: MdeModulePkg/Include/*NonDiscoverableDevice*.h
> > > +F: MdeModulePkg/Include/*NvmExpress*.h
> > > +F: MdeModulePkg/Include/*SerialPort*.h
> > > +F: MdeModulePkg/Include/*SdMmc*.h
> > > +F: MdeModulePkg/Include/*Ufs*.h
> > > +F: MdeModulePkg/Include/*Usb*.h
> > > +F: MdeModulePkg/Include/Guid/S3StorageDeviceInitList.h
> > > +F: MdeModulePkg/Include/Guid/RecoveryDevice.h
> > > +F: MdeModulePkg/Include/Library/PciHostBridgeLib.h
> > > +F: MdeModulePkg/Include/Ppi/StorageSecurityCommand.h
> > > +F: MdeModulePkg/Include/Protocol/Ps2Policy.h
> > > +R: Hao A Wu <hao.a.wu@intel.com>
> > > R: Ray Ni <ray.ni@intel.com>
> > > - (especially for Bus, Universal/Console, Universal/Disk,
> > > - Universal/BdsDxe and related libraries, header files)
> > > +
> > > +MdeModulePkg: Firmware Update modules
> > > +F: MdeModulePkg/*Capsule*/
> > > +F: MdeModulePkg/Library/DisplayUpdateProgressLib*/
> > > +F: MdeModulePkg/Library/FmpAuthenticationLibNull/
> > > +F: MdeModulePkg/Universal/Esrt*/
> > > +F: MdeModulePkg/Include/*Capsule*.h
> > > +F: MdeModulePkg/Include/Library/DisplayUpdateProgressLib.h
> > > +F: MdeModulePkg/Include/Library/FmpAuthenticationLib.h
> > > +F: MdeModulePkg/Include/Protocol/EsrtManagement.h
> > > +F: MdeModulePkg/Include/Protocol/FirmwareManagementProgress.h
> > > +R: Hao A Wu <hao.a.wu@intel.com>
> > > +R: Liming Gao <liming.gao@intel.com>
> > > +
> > > +MdeModulePkg: HII and UI modules
> > > +F: MdeModulePkg/*FileExplorer*/
> > > +F: MdeModulePkg/*Hii*/
> > > +F: MdeModulePkg/*Ui*/
> > > +F: MdeModulePkg/Application/BootManagerMenuApp/
> > > +F: MdeModulePkg/Library/CustomizedDisplayLib/
> > > +F: MdeModulePkg/Universal/DisplayEngineDxe/
> > > +F: MdeModulePkg/Universal/DriverSampleDxe/
> > > +F: MdeModulePkg/Universal/SetupBrowserDxe/
> > > +F: MdeModulePkg/Include/*FileExplorer*.h
> > > +F: MdeModulePkg/Include/*FormBrowser*.h
> > > +F: MdeModulePkg/Include/*Hii*.h
> > > +F: MdeModulePkg/Include/Library/CustomizedDisplayLib.h
> > > +F: MdeModulePkg/Include/Protocol/DisplayProtocol.h
> > > +R: Dandan Bi <dandan.bi@intel.com>
> > > +R: Eric Dong <eric.dong@intel.com>
> > > +
> > > +MdeModulePkg: Reset modules
> > > +F: MdeModulePkg/*Reset*/
> > > +F: MdeModulePkg/Include/*Reset*.h
> > > +R: Zhichao Gao <zhichao.gao@intel.com>
> > > +R: Ray Ni <ray.ni@intel.com>
> > > +
> > > +MdeModulePkg: S3 modules
> >
> > (4) Can we say "ACPI S3 modules"? (Not insisting, just suggesting.)
> >
> > > +F: MdeModulePkg/*LockBox*/
> > > +F: MdeModulePkg/Library/*S3*/
> > > +F: MdeModulePkg/Include/*BootScript*.h
> > > +F: MdeModulePkg/Include/*LockBox*.h
> > > +F: MdeModulePkg/Include/*S3*.h
> > > +R: Hao A Wu <hao.a.wu@intel.com>
> > > +R: Eric Dong <eric.dong@intel.com>
> > > +
> > > +MdeModulePkg: SMBIOS modules
> > > +F: MdeModulePkg/Universal/Smbios*/
> > > +R: Dandan Bi <dandan.bi@intel.com>
> > > R: Star Zeng <star.zeng@intel.com>
> > >
> > > +MdeModulePkg: SMM modules
> >
> > (5) Can we use the more generic term:
> >
> > Management Mode (MM, SMM) modules
> >
> > ?
> >
> > > +F: MdeModulePkg/*Smi*/
> > > +F: MdeModulePkg/*Smm*/
> > > +F: MdeModulePkg/Include/*Smi*.h
> > > +F: MdeModulePkg/Include/*Smm*.h
> > > +R: Eric Dong <eric.dong@intel.com>
> > > +R: Ray Ni <ray.ni@intel.com>
> > > +
> > > +MdeModulePkg: Variable modules
> >
> > (6) Can we say: "UEFI Variable modules"?
> >
> > > +F: MdeModulePkg/*Var*/
> > > +F: MdeModulePkg/Universal/FaultTolerantWrite*/
> > > +F: MdeModulePkg/Include/*/*FaultTolerantWrite*.h
> > > +F: MdeModulePkg/Include/*/*Var*.h
> > > +F: MdeModulePkg/Include/Guid/SystemNvDataGuid.h
> > > +F: MdeModulePkg/Include/Protocol/SwapAddressRange.h
> > > +R: Hao A Wu <hao.a.wu@intel.com>
> > > +R: Liming Gao <liming.gao@intel.com>
> > > +
> >
> > (7) Finally, a high level comment: personally, if I were listed as one
> > of the Reviewers, I'd prefer one patch per section added. But, that
> > preference is up to the Reviewers, of course.
> >
> > I guess the only point I'd really like to see fixed is (2) -- the
> > sorting of "F" patterns. The rest is optional, from my side.
> >
> > Thanks!
> > Laszlo
> >
> >
> > > MdePkg
> > > F: MdePkg/
> > > W: https://github.com/tianocore/tianocore.github.io/wiki/MdePkg
> > >
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] Maintainers.txt: Fine-grained review ownership for MdeModulePkg
2019-07-17 12:41 ` Laszlo Ersek
2019-07-17 14:36 ` Leif Lindholm
@ 2019-07-18 0:15 ` Wu, Hao A
1 sibling, 0 replies; 7+ messages in thread
From: Wu, Hao A @ 2019-07-18 0:15 UTC (permalink / raw)
To: devel@edk2.groups.io, lersek@redhat.com
Cc: Andrew Fish, Leif Lindholm, Kinney, Michael D, Bi, Dandan,
Dong, Eric, Gao, Liming, Ni, Ray, Zeng, Star, Gao, Zhichao
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Laszlo Ersek
> Sent: Wednesday, July 17, 2019 8:41 PM
> To: Wu, Hao A; devel@edk2.groups.io
> Cc: Andrew Fish; Leif Lindholm; Kinney, Michael D; Bi, Dandan; Dong, Eric;
> Gao, Liming; Ni, Ray; Zeng, Star; Gao, Zhichao
> Subject: Re: [edk2-devel] [PATCH v1 1/1] Maintainers.txt: Fine-grained
> review ownership for MdeModulePkg
>
> On 07/17/19 03:44, Hao A Wu wrote:
> > This commit add the reviewers information for modules within
> MdeModulePkg.
> >
> > For now the modules list includes:
> > ACPI
> > BDS
> > Console and Graphic
> > Core services (PEI, DXE and Runtime)
> > Device and Peripheral
> > Firmware Update
> > HII and UI
> > Reset
> > S3
> > SMBIOS
> > SMM
> > Variable
> >
> > Please note that, for MdeModulePkg components not included in the
> above
> > list, the reviewers will fall back to the package maintainers.
> >
> > Cc: Andrew Fish <afish@apple.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Leif Lindholm <leif.lindholm@linaro.org>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Dandan Bi <dandan.bi@intel.com>
> > Cc: Eric Dong <eric.dong@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> > Cc: Zhichao Gao <zhichao.gao@intel.com>
> > Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
> > ---
> > Maintainers.txt | 151 +++++++++++++++++++-
> > 1 file changed, 149 insertions(+), 2 deletions(-)
> >
> > diff --git a/Maintainers.txt b/Maintainers.txt
> > index 18fd2ef43c..c1ae02045e 100644
> > --- a/Maintainers.txt
> > +++ b/Maintainers.txt
> > @@ -186,11 +186,158 @@ F: MdeModulePkg/
> > W:
> https://github.com/tianocore/tianocore.github.io/wiki/MdeModulePkg
> > M: Jian J Wang <jian.j.wang@intel.com>
> > M: Hao A Wu <hao.a.wu@intel.com>
> > +
>
> (1) First comment here. This is actually a comment on the patch
> submission process, not the patch itself. I *suggest* (but do not
> *require*) that such changes be formatted for submission with at least
> -U6 (i.e. 6 lines of context, at the minimum). That makes the review
> much easier.
Agree. I will address this in the next patch series.
>
> Anyway, to explain the change to myself: basically this cuts the
> existent MdeModulePkg section in half, keeping Jian and Hao (both "M")
> assigned at the top-level, and pushing down all other folks ("R"s: Ray
> and Star) to other subsystems.
>
> That's fine.
>
>
> > +MdeModulePkg: ACPI modules
> > +F: MdeModulePkg/Universal/Acpi/
> > +F: MdeModulePkg/Include/*Acpi*.h
> > +R: Dandan Bi <dandan.bi@intel.com>
> > +R: Liming Gao <liming.gao@intel.com>
>
> Basic formatting (F followed by R) is fine.
>
> (2) General request (applies to all sections added in this patch): I
> suggest to sort the "F" patterns lexicographically. That will make
> future changes to the F patterns more localized and cleaner.
I will handle this.
For the proposed patch, I deliberately split the 'F:' list into 2 parts,
folders and header files. Each part are sorted alphabetically. In my
opinion, it makes the list a little bit cleaner.
But I will follow the comment here to keep consistency within this file.
>
> > +
> > +MdeModulePkg: BDS modules
> > +F: MdeModulePkg/*BootManager*/
> > +F: MdeModulePkg/Universal/BdsDxe/
> > +F: MdeModulePkg/Universal/LoadFileOnFv2/
> > +F:
> MdeModulePkg/Universal/SecurityStubDxe/Defer3rdPartyImageLoad.*
> > +F: MdeModulePkg/Universal/DevicePathDxe/
> > +F: MdeModulePkg/Universal/DriverHealthManagerDxe/
> > +F: MdeModulePkg/Include/Library/UefiBootManagerLib.h
> > +R: Zhichao Gao <zhichao.gao@intel.com>
> > +R: Ray Ni <ray.ni@intel.com>
> > +
> > +MdeModulePkg: Console and Graphic modules
>
> (3) Suggesting a typo fix: "Graphic*s*" modules. "Graphic modules"
> suggests they are modules restricted from an under-aged audience. :)
Agree. 'Graphics' is also used in the name of modules under MdeModulePkg.
>
> (I'm not a native English speaker though.)
>
>
> > +F: MdeModulePkg/*Logo*/
> > +F: MdeModulePkg/Library/BaseBmpSupportLib/
> > +F: MdeModulePkg/Library/FrameBufferBltLib/
> > +F: MdeModulePkg/Universal/Console/
> > +F: MdeModulePkg/Include/*Logo*.h
> > +F: MdeModulePkg/Include/Guid/ConnectConInEvent.h
> > +F: MdeModulePkg/Include/Guid/Console*.h
> > +F: MdeModulePkg/Include/Guid/StandardErrorDevice.h
> > +F: MdeModulePkg/Include/Guid/TtyTerm.h
> > +F: MdeModulePkg/Include/Library/BmpSupportLib.h
> > +F: MdeModulePkg/Include/Library/FrameBufferBltLib.h
> > +R: Zhichao Gao <zhichao.gao@intel.com>
> > +R: Ray Ni <ray.ni@intel.com>
> > +
> > +MdeModulePkg: Core services (PEI, DXE and Runtime) modules
> > +F: MdeModulePkg/*Mem*/
> > +F: MdeModulePkg/*SectionExtract*/
> > +F: MdeModulePkg/*StatusCode*/
> > +F: MdeModulePkg/Application/DumpDynPcd/
> > +F: MdeModulePkg/Core/Dxe/
> > +F: MdeModulePkg/Core/DxeIplPeim/
> > +F: MdeModulePkg/Core/Pei/
> > +F: MdeModulePkg/Core/RuntimeDxe/
> > +F: MdeModulePkg/Library/*Decompress*/
> > +F: MdeModulePkg/Library/*Perf*/
> > +F: MdeModulePkg/Library/DxeSecurityManagementLib/
> > +F: MdeModulePkg/Universal/PCD/
> > +F: MdeModulePkg/Universal/PlatformDriOverrideDxe/
> > +F: MdeModulePkg/Universal/SecurityStubDxe/SecurityStub.c
> > +F: MdeModulePkg/Include/*Mem*.h
> > +F: MdeModulePkg/Include/*Pcd*.h
> > +F: MdeModulePkg/Include/*Perf*.h
> > +F: MdeModulePkg/Include/*StatusCode*.h
> > +F: MdeModulePkg/Include/Guid/Crc32GuidedSectionExtraction.h
> > +F: MdeModulePkg/Include/Guid/EventExitBootServiceFailed.h
> > +F: MdeModulePkg/Include/Guid/IdleLoopEvent.h
> > +F: MdeModulePkg/Include/Guid/LoadModuleAtFixedAddress.h
> > +F: MdeModulePkg/Include/Guid/LzmaDecompress.h
> > +F: MdeModulePkg/Include/Library/SecurityManagementLib.h
> > +R: Dandan Bi <dandan.bi@intel.com>
> > +R: Liming Gao <liming.gao@intel.com>
> > +
> > +MdeModulePkg: Device and Peripheral modules
> > +F: MdeModulePkg/*PciHostBridge*/
> > +F: MdeModulePkg/*Serial*/
> > +F: MdeModulePkg/Bus/
> > +F: MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/
> > +F: MdeModulePkg/Universal/Disk/
> > +F: MdeModulePkg/Universal/PcatSingleSegmentPciCfg2Pei/
> > +F: MdeModulePkg/Include/*Ata*.h
> > +F: MdeModulePkg/Include/*IoMmu*.h
> > +F: MdeModulePkg/Include/*NonDiscoverableDevice*.h
> > +F: MdeModulePkg/Include/*NvmExpress*.h
> > +F: MdeModulePkg/Include/*SerialPort*.h
> > +F: MdeModulePkg/Include/*SdMmc*.h
> > +F: MdeModulePkg/Include/*Ufs*.h
> > +F: MdeModulePkg/Include/*Usb*.h
> > +F: MdeModulePkg/Include/Guid/S3StorageDeviceInitList.h
> > +F: MdeModulePkg/Include/Guid/RecoveryDevice.h
> > +F: MdeModulePkg/Include/Library/PciHostBridgeLib.h
> > +F: MdeModulePkg/Include/Ppi/StorageSecurityCommand.h
> > +F: MdeModulePkg/Include/Protocol/Ps2Policy.h
> > +R: Hao A Wu <hao.a.wu@intel.com>
> > R: Ray Ni <ray.ni@intel.com>
> > - (especially for Bus, Universal/Console, Universal/Disk,
> > - Universal/BdsDxe and related libraries, header files)
> > +
> > +MdeModulePkg: Firmware Update modules
> > +F: MdeModulePkg/*Capsule*/
> > +F: MdeModulePkg/Library/DisplayUpdateProgressLib*/
> > +F: MdeModulePkg/Library/FmpAuthenticationLibNull/
> > +F: MdeModulePkg/Universal/Esrt*/
> > +F: MdeModulePkg/Include/*Capsule*.h
> > +F: MdeModulePkg/Include/Library/DisplayUpdateProgressLib.h
> > +F: MdeModulePkg/Include/Library/FmpAuthenticationLib.h
> > +F: MdeModulePkg/Include/Protocol/EsrtManagement.h
> > +F: MdeModulePkg/Include/Protocol/FirmwareManagementProgress.h
> > +R: Hao A Wu <hao.a.wu@intel.com>
> > +R: Liming Gao <liming.gao@intel.com>
> > +
> > +MdeModulePkg: HII and UI modules
> > +F: MdeModulePkg/*FileExplorer*/
> > +F: MdeModulePkg/*Hii*/
> > +F: MdeModulePkg/*Ui*/
> > +F: MdeModulePkg/Application/BootManagerMenuApp/
> > +F: MdeModulePkg/Library/CustomizedDisplayLib/
> > +F: MdeModulePkg/Universal/DisplayEngineDxe/
> > +F: MdeModulePkg/Universal/DriverSampleDxe/
> > +F: MdeModulePkg/Universal/SetupBrowserDxe/
> > +F: MdeModulePkg/Include/*FileExplorer*.h
> > +F: MdeModulePkg/Include/*FormBrowser*.h
> > +F: MdeModulePkg/Include/*Hii*.h
> > +F: MdeModulePkg/Include/Library/CustomizedDisplayLib.h
> > +F: MdeModulePkg/Include/Protocol/DisplayProtocol.h
> > +R: Dandan Bi <dandan.bi@intel.com>
> > +R: Eric Dong <eric.dong@intel.com>
> > +
> > +MdeModulePkg: Reset modules
> > +F: MdeModulePkg/*Reset*/
> > +F: MdeModulePkg/Include/*Reset*.h
> > +R: Zhichao Gao <zhichao.gao@intel.com>
> > +R: Ray Ni <ray.ni@intel.com>
> > +
> > +MdeModulePkg: S3 modules
>
> (4) Can we say "ACPI S3 modules"? (Not insisting, just suggesting.)
I am fine with that. Will rename this one.
>
> > +F: MdeModulePkg/*LockBox*/
> > +F: MdeModulePkg/Library/*S3*/
> > +F: MdeModulePkg/Include/*BootScript*.h
> > +F: MdeModulePkg/Include/*LockBox*.h
> > +F: MdeModulePkg/Include/*S3*.h
> > +R: Hao A Wu <hao.a.wu@intel.com>
> > +R: Eric Dong <eric.dong@intel.com>
> > +
> > +MdeModulePkg: SMBIOS modules
> > +F: MdeModulePkg/Universal/Smbios*/
> > +R: Dandan Bi <dandan.bi@intel.com>
> > R: Star Zeng <star.zeng@intel.com>
> >
> > +MdeModulePkg: SMM modules
>
> (5) Can we use the more generic term:
>
> Management Mode (MM, SMM) modules
Yes. The suggested name is better.
>
> ?
>
> > +F: MdeModulePkg/*Smi*/
> > +F: MdeModulePkg/*Smm*/
> > +F: MdeModulePkg/Include/*Smi*.h
> > +F: MdeModulePkg/Include/*Smm*.h
> > +R: Eric Dong <eric.dong@intel.com>
> > +R: Ray Ni <ray.ni@intel.com>
> > +
> > +MdeModulePkg: Variable modules
>
> (6) Can we say: "UEFI Variable modules"?
Yes.
Best Regards,
Hao Wu
>
> > +F: MdeModulePkg/*Var*/
> > +F: MdeModulePkg/Universal/FaultTolerantWrite*/
> > +F: MdeModulePkg/Include/*/*FaultTolerantWrite*.h
> > +F: MdeModulePkg/Include/*/*Var*.h
> > +F: MdeModulePkg/Include/Guid/SystemNvDataGuid.h
> > +F: MdeModulePkg/Include/Protocol/SwapAddressRange.h
> > +R: Hao A Wu <hao.a.wu@intel.com>
> > +R: Liming Gao <liming.gao@intel.com>
> > +
>
> (7) Finally, a high level comment: personally, if I were listed as one
> of the Reviewers, I'd prefer one patch per section added. But, that
> preference is up to the Reviewers, of course.
>
> I guess the only point I'd really like to see fixed is (2) -- the
> sorting of "F" patterns. The rest is optional, from my side.
>
> Thanks!
> Laszlo
>
>
> > MdePkg
> > F: MdePkg/
> > W: https://github.com/tianocore/tianocore.github.io/wiki/MdePkg
> >
>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] Maintainers.txt: Fine-grained review ownership for MdeModulePkg
2019-07-17 14:36 ` Leif Lindholm
2019-07-17 15:19 ` Ni, Ray
@ 2019-07-18 5:50 ` Wu, Hao A
1 sibling, 0 replies; 7+ messages in thread
From: Wu, Hao A @ 2019-07-18 5:50 UTC (permalink / raw)
To: devel@edk2.groups.io, leif.lindholm@linaro.org, Laszlo Ersek
Cc: Andrew Fish, Kinney, Michael D, Bi, Dandan, Dong, Eric,
Gao, Liming, Ni, Ray, Zeng, Star, Gao, Zhichao
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Leif Lindholm
> Sent: Wednesday, July 17, 2019 10:37 PM
> To: Laszlo Ersek
> Cc: Wu, Hao A; devel@edk2.groups.io; Andrew Fish; Kinney, Michael D; Bi,
> Dandan; Dong, Eric; Gao, Liming; Ni, Ray; Zeng, Star; Gao, Zhichao
> Subject: Re: [edk2-devel] [PATCH v1 1/1] Maintainers.txt: Fine-grained
> review ownership for MdeModulePkg
>
> Top-posting because lazy.
>
> Basically, I agree with all of Laszlo's comments.
> I would be happy for this to remain a single patch, however. Partly
> because I'm lazy. But also because this is effectively "converting"
> a single package (MdeModulePkg) into multiple sections. (This is not a
> strong preference.)
>
> I would however prefer a v2 with sorted F:-patterns.
Hello Leif,
I have sent the V2 patch to address most of Laszlo's comments, except (7).
For (7), I still keep the change within a single patch.
So far, the patch has received the R-B tag for all the module reviewers in
MdeModulePkg proposed by this patch.
Please let me know if there is anything I should do before the patch can
be pushed, thanks.
Best Regards,
Hao Wu
>
> And of course, Acks/Reviews from everyone affected.
>
> Best Regards,
>
> Leif
>
> On Wed, Jul 17, 2019 at 02:41:15PM +0200, Laszlo Ersek wrote:
> > On 07/17/19 03:44, Hao A Wu wrote:
> > > This commit add the reviewers information for modules within
> MdeModulePkg.
> > >
> > > For now the modules list includes:
> > > ACPI
> > > BDS
> > > Console and Graphic
> > > Core services (PEI, DXE and Runtime)
> > > Device and Peripheral
> > > Firmware Update
> > > HII and UI
> > > Reset
> > > S3
> > > SMBIOS
> > > SMM
> > > Variable
> > >
> > > Please note that, for MdeModulePkg components not included in the
> above
> > > list, the reviewers will fall back to the package maintainers.
> > >
> > > Cc: Andrew Fish <afish@apple.com>
> > > Cc: Laszlo Ersek <lersek@redhat.com>
> > > Cc: Leif Lindholm <leif.lindholm@linaro.org>
> > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > Cc: Dandan Bi <dandan.bi@intel.com>
> > > Cc: Eric Dong <eric.dong@intel.com>
> > > Cc: Liming Gao <liming.gao@intel.com>
> > > Cc: Ray Ni <ray.ni@intel.com>
> > > Cc: Star Zeng <star.zeng@intel.com>
> > > Cc: Zhichao Gao <zhichao.gao@intel.com>
> > > Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
> > > ---
> > > Maintainers.txt | 151 +++++++++++++++++++-
> > > 1 file changed, 149 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Maintainers.txt b/Maintainers.txt
> > > index 18fd2ef43c..c1ae02045e 100644
> > > --- a/Maintainers.txt
> > > +++ b/Maintainers.txt
> > > @@ -186,11 +186,158 @@ F: MdeModulePkg/
> > > W:
> https://github.com/tianocore/tianocore.github.io/wiki/MdeModulePkg
> > > M: Jian J Wang <jian.j.wang@intel.com>
> > > M: Hao A Wu <hao.a.wu@intel.com>
> > > +
> >
> > (1) First comment here. This is actually a comment on the patch
> > submission process, not the patch itself. I *suggest* (but do not
> > *require*) that such changes be formatted for submission with at least
> > -U6 (i.e. 6 lines of context, at the minimum). That makes the review
> > much easier.
> >
> > Anyway, to explain the change to myself: basically this cuts the
> > existent MdeModulePkg section in half, keeping Jian and Hao (both "M")
> > assigned at the top-level, and pushing down all other folks ("R"s: Ray
> > and Star) to other subsystems.
> >
> > That's fine.
> >
> >
> > > +MdeModulePkg: ACPI modules
> > > +F: MdeModulePkg/Universal/Acpi/
> > > +F: MdeModulePkg/Include/*Acpi*.h
> > > +R: Dandan Bi <dandan.bi@intel.com>
> > > +R: Liming Gao <liming.gao@intel.com>
> >
> > Basic formatting (F followed by R) is fine.
> >
> > (2) General request (applies to all sections added in this patch): I
> > suggest to sort the "F" patterns lexicographically. That will make
> > future changes to the F patterns more localized and cleaner.
> >
> > > +
> > > +MdeModulePkg: BDS modules
> > > +F: MdeModulePkg/*BootManager*/
> > > +F: MdeModulePkg/Universal/BdsDxe/
> > > +F: MdeModulePkg/Universal/LoadFileOnFv2/
> > > +F:
> MdeModulePkg/Universal/SecurityStubDxe/Defer3rdPartyImageLoad.*
> > > +F: MdeModulePkg/Universal/DevicePathDxe/
> > > +F: MdeModulePkg/Universal/DriverHealthManagerDxe/
> > > +F: MdeModulePkg/Include/Library/UefiBootManagerLib.h
> > > +R: Zhichao Gao <zhichao.gao@intel.com>
> > > +R: Ray Ni <ray.ni@intel.com>
> > > +
> > > +MdeModulePkg: Console and Graphic modules
> >
> > (3) Suggesting a typo fix: "Graphic*s*" modules. "Graphic modules"
> > suggests they are modules restricted from an under-aged audience. :)
> >
> > (I'm not a native English speaker though.)
> >
> >
> > > +F: MdeModulePkg/*Logo*/
> > > +F: MdeModulePkg/Library/BaseBmpSupportLib/
> > > +F: MdeModulePkg/Library/FrameBufferBltLib/
> > > +F: MdeModulePkg/Universal/Console/
> > > +F: MdeModulePkg/Include/*Logo*.h
> > > +F: MdeModulePkg/Include/Guid/ConnectConInEvent.h
> > > +F: MdeModulePkg/Include/Guid/Console*.h
> > > +F: MdeModulePkg/Include/Guid/StandardErrorDevice.h
> > > +F: MdeModulePkg/Include/Guid/TtyTerm.h
> > > +F: MdeModulePkg/Include/Library/BmpSupportLib.h
> > > +F: MdeModulePkg/Include/Library/FrameBufferBltLib.h
> > > +R: Zhichao Gao <zhichao.gao@intel.com>
> > > +R: Ray Ni <ray.ni@intel.com>
> > > +
> > > +MdeModulePkg: Core services (PEI, DXE and Runtime) modules
> > > +F: MdeModulePkg/*Mem*/
> > > +F: MdeModulePkg/*SectionExtract*/
> > > +F: MdeModulePkg/*StatusCode*/
> > > +F: MdeModulePkg/Application/DumpDynPcd/
> > > +F: MdeModulePkg/Core/Dxe/
> > > +F: MdeModulePkg/Core/DxeIplPeim/
> > > +F: MdeModulePkg/Core/Pei/
> > > +F: MdeModulePkg/Core/RuntimeDxe/
> > > +F: MdeModulePkg/Library/*Decompress*/
> > > +F: MdeModulePkg/Library/*Perf*/
> > > +F: MdeModulePkg/Library/DxeSecurityManagementLib/
> > > +F: MdeModulePkg/Universal/PCD/
> > > +F: MdeModulePkg/Universal/PlatformDriOverrideDxe/
> > > +F: MdeModulePkg/Universal/SecurityStubDxe/SecurityStub.c
> > > +F: MdeModulePkg/Include/*Mem*.h
> > > +F: MdeModulePkg/Include/*Pcd*.h
> > > +F: MdeModulePkg/Include/*Perf*.h
> > > +F: MdeModulePkg/Include/*StatusCode*.h
> > > +F: MdeModulePkg/Include/Guid/Crc32GuidedSectionExtraction.h
> > > +F: MdeModulePkg/Include/Guid/EventExitBootServiceFailed.h
> > > +F: MdeModulePkg/Include/Guid/IdleLoopEvent.h
> > > +F: MdeModulePkg/Include/Guid/LoadModuleAtFixedAddress.h
> > > +F: MdeModulePkg/Include/Guid/LzmaDecompress.h
> > > +F: MdeModulePkg/Include/Library/SecurityManagementLib.h
> > > +R: Dandan Bi <dandan.bi@intel.com>
> > > +R: Liming Gao <liming.gao@intel.com>
> > > +
> > > +MdeModulePkg: Device and Peripheral modules
> > > +F: MdeModulePkg/*PciHostBridge*/
> > > +F: MdeModulePkg/*Serial*/
> > > +F: MdeModulePkg/Bus/
> > > +F: MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/
> > > +F: MdeModulePkg/Universal/Disk/
> > > +F: MdeModulePkg/Universal/PcatSingleSegmentPciCfg2Pei/
> > > +F: MdeModulePkg/Include/*Ata*.h
> > > +F: MdeModulePkg/Include/*IoMmu*.h
> > > +F: MdeModulePkg/Include/*NonDiscoverableDevice*.h
> > > +F: MdeModulePkg/Include/*NvmExpress*.h
> > > +F: MdeModulePkg/Include/*SerialPort*.h
> > > +F: MdeModulePkg/Include/*SdMmc*.h
> > > +F: MdeModulePkg/Include/*Ufs*.h
> > > +F: MdeModulePkg/Include/*Usb*.h
> > > +F: MdeModulePkg/Include/Guid/S3StorageDeviceInitList.h
> > > +F: MdeModulePkg/Include/Guid/RecoveryDevice.h
> > > +F: MdeModulePkg/Include/Library/PciHostBridgeLib.h
> > > +F: MdeModulePkg/Include/Ppi/StorageSecurityCommand.h
> > > +F: MdeModulePkg/Include/Protocol/Ps2Policy.h
> > > +R: Hao A Wu <hao.a.wu@intel.com>
> > > R: Ray Ni <ray.ni@intel.com>
> > > - (especially for Bus, Universal/Console, Universal/Disk,
> > > - Universal/BdsDxe and related libraries, header files)
> > > +
> > > +MdeModulePkg: Firmware Update modules
> > > +F: MdeModulePkg/*Capsule*/
> > > +F: MdeModulePkg/Library/DisplayUpdateProgressLib*/
> > > +F: MdeModulePkg/Library/FmpAuthenticationLibNull/
> > > +F: MdeModulePkg/Universal/Esrt*/
> > > +F: MdeModulePkg/Include/*Capsule*.h
> > > +F: MdeModulePkg/Include/Library/DisplayUpdateProgressLib.h
> > > +F: MdeModulePkg/Include/Library/FmpAuthenticationLib.h
> > > +F: MdeModulePkg/Include/Protocol/EsrtManagement.h
> > > +F: MdeModulePkg/Include/Protocol/FirmwareManagementProgress.h
> > > +R: Hao A Wu <hao.a.wu@intel.com>
> > > +R: Liming Gao <liming.gao@intel.com>
> > > +
> > > +MdeModulePkg: HII and UI modules
> > > +F: MdeModulePkg/*FileExplorer*/
> > > +F: MdeModulePkg/*Hii*/
> > > +F: MdeModulePkg/*Ui*/
> > > +F: MdeModulePkg/Application/BootManagerMenuApp/
> > > +F: MdeModulePkg/Library/CustomizedDisplayLib/
> > > +F: MdeModulePkg/Universal/DisplayEngineDxe/
> > > +F: MdeModulePkg/Universal/DriverSampleDxe/
> > > +F: MdeModulePkg/Universal/SetupBrowserDxe/
> > > +F: MdeModulePkg/Include/*FileExplorer*.h
> > > +F: MdeModulePkg/Include/*FormBrowser*.h
> > > +F: MdeModulePkg/Include/*Hii*.h
> > > +F: MdeModulePkg/Include/Library/CustomizedDisplayLib.h
> > > +F: MdeModulePkg/Include/Protocol/DisplayProtocol.h
> > > +R: Dandan Bi <dandan.bi@intel.com>
> > > +R: Eric Dong <eric.dong@intel.com>
> > > +
> > > +MdeModulePkg: Reset modules
> > > +F: MdeModulePkg/*Reset*/
> > > +F: MdeModulePkg/Include/*Reset*.h
> > > +R: Zhichao Gao <zhichao.gao@intel.com>
> > > +R: Ray Ni <ray.ni@intel.com>
> > > +
> > > +MdeModulePkg: S3 modules
> >
> > (4) Can we say "ACPI S3 modules"? (Not insisting, just suggesting.)
> >
> > > +F: MdeModulePkg/*LockBox*/
> > > +F: MdeModulePkg/Library/*S3*/
> > > +F: MdeModulePkg/Include/*BootScript*.h
> > > +F: MdeModulePkg/Include/*LockBox*.h
> > > +F: MdeModulePkg/Include/*S3*.h
> > > +R: Hao A Wu <hao.a.wu@intel.com>
> > > +R: Eric Dong <eric.dong@intel.com>
> > > +
> > > +MdeModulePkg: SMBIOS modules
> > > +F: MdeModulePkg/Universal/Smbios*/
> > > +R: Dandan Bi <dandan.bi@intel.com>
> > > R: Star Zeng <star.zeng@intel.com>
> > >
> > > +MdeModulePkg: SMM modules
> >
> > (5) Can we use the more generic term:
> >
> > Management Mode (MM, SMM) modules
> >
> > ?
> >
> > > +F: MdeModulePkg/*Smi*/
> > > +F: MdeModulePkg/*Smm*/
> > > +F: MdeModulePkg/Include/*Smi*.h
> > > +F: MdeModulePkg/Include/*Smm*.h
> > > +R: Eric Dong <eric.dong@intel.com>
> > > +R: Ray Ni <ray.ni@intel.com>
> > > +
> > > +MdeModulePkg: Variable modules
> >
> > (6) Can we say: "UEFI Variable modules"?
> >
> > > +F: MdeModulePkg/*Var*/
> > > +F: MdeModulePkg/Universal/FaultTolerantWrite*/
> > > +F: MdeModulePkg/Include/*/*FaultTolerantWrite*.h
> > > +F: MdeModulePkg/Include/*/*Var*.h
> > > +F: MdeModulePkg/Include/Guid/SystemNvDataGuid.h
> > > +F: MdeModulePkg/Include/Protocol/SwapAddressRange.h
> > > +R: Hao A Wu <hao.a.wu@intel.com>
> > > +R: Liming Gao <liming.gao@intel.com>
> > > +
> >
> > (7) Finally, a high level comment: personally, if I were listed as one
> > of the Reviewers, I'd prefer one patch per section added. But, that
> > preference is up to the Reviewers, of course.
> >
> > I guess the only point I'd really like to see fixed is (2) -- the
> > sorting of "F" patterns. The rest is optional, from my side.
> >
> > Thanks!
> > Laszlo
> >
> >
> > > MdePkg
> > > F: MdePkg/
> > > W: https://github.com/tianocore/tianocore.github.io/wiki/MdePkg
> > >
> >
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-07-18 5:50 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-07-17 1:44 [PATCH v1 0/1] Maintainers.txt: Review ownership for MdeModulePkg Wu, Hao A
2019-07-17 1:44 ` [PATCH v1 1/1] Maintainers.txt: Fine-grained review " Wu, Hao A
2019-07-17 12:41 ` Laszlo Ersek
2019-07-17 14:36 ` Leif Lindholm
2019-07-17 15:19 ` Ni, Ray
2019-07-18 5:50 ` [edk2-devel] " Wu, Hao A
2019-07-18 0:15 ` Wu, Hao A
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox