Re: [PATCH v1 1/1] Maintainers.txt: Fine-grained review ownership for MdeModulePkg


Laszlo Ersek
 

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@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Leif Lindholm <leif.lindholm@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Dandan Bi <dandan.bi@...>
Cc: Eric Dong <eric.dong@...>
Cc: Liming Gao <liming.gao@...>
Cc: Ray Ni <ray.ni@...>
Cc: Star Zeng <star.zeng@...>
Cc: Zhichao Gao <zhichao.gao@...>
Signed-off-by: Hao A Wu <hao.a.wu@...>
---
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@...>
M: Hao A Wu <hao.a.wu@...>
+
(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@...>
+R: Liming Gao <liming.gao@...>
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@...>
+R: Ray Ni <ray.ni@...>
+
+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@...>
+R: Ray Ni <ray.ni@...>
+
+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@...>
+R: Liming Gao <liming.gao@...>
+
+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@...>
R: Ray Ni <ray.ni@...>
- (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@...>
+R: Liming Gao <liming.gao@...>
+
+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@...>
+R: Eric Dong <eric.dong@...>
+
+MdeModulePkg: Reset modules
+F: MdeModulePkg/*Reset*/
+F: MdeModulePkg/Include/*Reset*.h
+R: Zhichao Gao <zhichao.gao@...>
+R: Ray Ni <ray.ni@...>
+
+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@...>
+R: Eric Dong <eric.dong@...>
+
+MdeModulePkg: SMBIOS modules
+F: MdeModulePkg/Universal/Smbios*/
+R: Dandan Bi <dandan.bi@...>
R: Star Zeng <star.zeng@...>

+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@...>
+R: Ray Ni <ray.ni@...>
+
+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@...>
+R: Liming Gao <liming.gao@...>
+
(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

Join devel@edk2.groups.io to automatically receive all group messages.