From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=rXXsn3W/; spf=pass (domain: linaro.org, ip: 209.85.221.65, mailfrom: leif.lindholm@linaro.org) Received: from mail-wr1-f65.google.com (mail-wr1-f65.google.com [209.85.221.65]) by groups.io with SMTP; Wed, 17 Jul 2019 07:36:51 -0700 Received: by mail-wr1-f65.google.com with SMTP id n9so25166654wru.0 for ; Wed, 17 Jul 2019 07:36:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=rbrj4jX4VuyTttnQTKknwYvgFex2OTtF3f0vj8UT9xU=; b=rXXsn3W/XKDCI3sG9cIifInysRG1A4QpVwNq4Y7r/JIKdE3cPnGYm4a/SbAz7TU1Aw A9lNKqa4cWHJyfvkhpaJODWPL4x4Gka+/tEsE8bysOxRR+TnnCNekj65/qfHhY7myIWs twj0UfSH90KPZ5F8Pk+TPMMdrhWezMS3TL5g8HOlpGFzKytmQxvjXKKCwh+dTjxsokgb M2z/O7IQIesZNOFN2HWFm0btp9ZOGGpZXCIfweb5U1W7Zz1hWQeX/h08+fc9qfnS9GAL +lWan8/Xs3twcJzXiRzHxvrBS5Un2Z08sw6EpLKKqlxo5shGJJI2n8kaZ2BbPrW/X56H tmQg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=rbrj4jX4VuyTttnQTKknwYvgFex2OTtF3f0vj8UT9xU=; b=J3efQkpMUdyFPxPUy7MKRstHHHfse6ZqYgXeiROn8vyUz06PgH8Ag66Nrt61QqW1ox OPzMY8Bnh1kOOvftW7RdHSamSp5hg3NDVVpbzLFOm14V1WQte/gn75+ibTINbStL8yhk +Fev9UfpVdUldQMwmBpXHqHMhRuEl0gJp/u2sU3WNDhTfAVO47QnnBWTbR+tEHo66Xmt +PrpJI6HhHIZzbkE4woMNR2ztJ3+AVKYM544gQnlFrnaJ0rQXdAyBWqAkOopSr0i0NZl VdUkmDGmILIKkeJk6yK956FeLVI0qN/B80ObRbL32JMcDmAHUfp/W4dTgvnURsIQAWjG WiPQ== X-Gm-Message-State: APjAAAV0iPZ+a/Zjg8XMzR53jH6WBLchoeUl4SBRsBeRfhPZ0RwGQNu5 QUGLx/GfDJBPTFvyulhTb7S08g== X-Google-Smtp-Source: APXvYqxKVKa6qREiwglj/S24XD1e6pP9r/B2tOcZR+GCMNJ+QLXHngDCv3Trl8l6o0jAZIVikq9c7g== X-Received: by 2002:a5d:4090:: with SMTP id o16mr44768023wrp.292.1563374209593; Wed, 17 Jul 2019 07:36:49 -0700 (PDT) Return-Path: Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id p18sm21803456wrm.16.2019.07.17.07.36.48 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Wed, 17 Jul 2019 07:36:48 -0700 (PDT) Date: Wed, 17 Jul 2019 15:36:47 +0100 From: "Leif Lindholm" To: Laszlo Ersek Cc: Hao A Wu , devel@edk2.groups.io, Andrew Fish , Michael D Kinney , Dandan Bi , Eric Dong , Liming Gao , Ray Ni , Star Zeng , Zhichao Gao Subject: Re: [PATCH v1 1/1] Maintainers.txt: Fine-grained review ownership for MdeModulePkg Message-ID: <20190717143647.GC2712@bivouac.eciton.net> References: <20190717014437.19944-1-hao.a.wu@intel.com> <20190717014437.19944-2-hao.a.wu@intel.com> <665a5165-b859-a3bd-82b7-278e9c01471e@redhat.com> MIME-Version: 1.0 In-Reply-To: <665a5165-b859-a3bd-82b7-278e9c01471e@redhat.com> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 > > 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 > > >