From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.65, mailfrom: ray.ni@intel.com) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by groups.io with SMTP; Wed, 17 Jul 2019 08:19:20 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 17 Jul 2019 08:19:19 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,274,1559545200"; d="scan'208";a="167985238" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by fmsmga008.fm.intel.com with ESMTP; 17 Jul 2019 08:19:18 -0700 Received: from fmsmsx125.amr.corp.intel.com (10.18.125.40) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 17 Jul 2019 08:19:18 -0700 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by FMSMSX125.amr.corp.intel.com (10.18.125.40) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 17 Jul 2019 08:19:18 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.110]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.55]) with mapi id 14.03.0439.000; Wed, 17 Jul 2019 23:19:16 +0800 From: "Ni, Ray" 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" Subject: Re: [PATCH v1 1/1] Maintainers.txt: Fine-grained review ownership for MdeModulePkg Thread-Topic: [PATCH v1 1/1] Maintainers.txt: Fine-grained review ownership for MdeModulePkg Thread-Index: AQHVPK0VUpYBwX9j9U6auObX8z8ikKbO7FuA Date: Wed, 17 Jul 2019 15:19:15 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D5C233E14@SHSMSX104.ccr.corp.intel.com> References: <20190717014437.19944-1-hao.a.wu@intel.com> <20190717014437.19944-2-hao.a.wu@intel.com> <665a5165-b859-a3bd-82b7-278e9c01471e@redhat.com> <20190717143647.GC2712@bivouac.eciton.net> In-Reply-To: <20190717143647.GC2712@bivouac.eciton.net> Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: ray.ni@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Regarding the role assigned to me, Reviewed-by: Ray Ni > -----Original Message----- > From: 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: [PATCH v1 1/1] Maintainers.txt: Fine-grained review ownershi= p for > MdeModulePkg >=20 > Top-posting because lazy. >=20 > Basically, I agree with all of Laszlo's comments. > I would be happy for this to remain a single patch, however. Partly becau= se I'm > lazy. But also because this is effectively "converting" > a single package (MdeModulePkg) into multiple sections. (This is not a st= rong > preference.) >=20 > I would however prefer a v2 with sorted F:-patterns. >=20 > And of course, Acks/Reviews from everyone affected. >=20 > Best Regards, >=20 > Leif >=20 > 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 > > > Cc: Laszlo Ersek > > > Cc: Leif Lindholm > > > Cc: Michael D Kinney > > > Cc: Dandan Bi > > > Cc: Eric Dong > > > Cc: Liming Gao > > > Cc: Ray Ni > > > Cc: Star Zeng > > > Cc: Zhichao Gao > > > Signed-off-by: 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 > > > M: 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 > > > +R: 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 > > > +R: 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 > > > +R: 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 > > > +R: 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 > > > R: 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 > > > +R: 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 > > > +R: Eric Dong > > > + > > > +MdeModulePkg: Reset modules > > > +F: MdeModulePkg/*Reset*/ > > > +F: MdeModulePkg/Include/*Reset*.h > > > +R: Zhichao Gao > > > +R: 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 > > > +R: Eric Dong > > > + > > > +MdeModulePkg: SMBIOS modules > > > +F: MdeModulePkg/Universal/Smbios*/ > > > +R: Dandan Bi > > > R: 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 > > > +R: 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 > > > +R: 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 > > > > >