public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Yao, Jiewen" <jiewen.yao@intel.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
	"lersek@redhat.com" <lersek@redhat.com>,
	"pedro.falcato@gmail.com" <pedro.falcato@gmail.com>,
	"Kinney, Michael D" <michael.d.kinney@intel.com>
Cc: Andrew Fish <afish@apple.com>,
	Leif Lindholm <quic_llindhol@quicinc.com>,
	"Warkentin, Andrei" <andrei.warkentin@intel.com>,
	"West, Catharine" <catharine.west@intel.com>,
	"Bi, Dandan" <dandan.bi@intel.com>,
	"Daniel Schaefer" <git@danielschaefer.me>,
	David Woodhouse <dwmw2@infradead.org>,
	"De, Debkumar" <debkumar.de@intel.com>,
	"Dong, Eric" <eric.dong@intel.com>,
	"Jiang, Guomin" <guomin.jiang@intel.com>,
	"Wu, Hao A" <hao.a.wu@intel.com>,
	James Bottomley <jejb@linux.ibm.com>,
	"Wang, Jian J" <jian.j.wang@intel.com>,
	"Justen, Jordan L" <jordan.l.justen@intel.com>,
	Julien Grall <julien@xen.org>, Peter Grehan <grehan@freebsd.org>,
	"Zhang, Qi1" <qi1.zhang@intel.com>,
	"Ng, Ray Han Lim" <ray.han.lim.ng@intel.com>,
	"Stefan Berger" <stefanb@linux.ibm.com>,
	"Hou, Wenxing" <wenxing.hou@intel.com>,
	"Lu, Xiaoyu1" <xiaoyu1.lu@intel.com>
Subject: Re: [edk2-devel] [Patch 1/1] Maintainers.txt: Update based on active community members
Date: Mon, 30 Oct 2023 02:54:10 +0000	[thread overview]
Message-ID: <MW4PR11MB5872D65D4E889737C49C98538CA1A@MW4PR11MB5872.namprd11.prod.outlook.com> (raw)
In-Reply-To: <da903fdb-92d1-ccb8-1aba-eeb301412a3d@redhat.com>

> I'll re-raise my point about relaxing the contribution conditions too --
> given this state, I'd propose a "merge by default" approach, with a
> reasonable timeout.

[Jiewen] Yes. I agree this approach.
A reasonable timeout seems enough to allow people to think and feedback.



Also, I would like to propose another the contribution condition relax.

Currently, our agreed condition to merge is:
1) Reviewed-by from Maintainer.
2) Acked-by from Maintainer + Reviewed-by from Reviewer

I propose to change the second condition:
2) Acked-by from Maintainer + Reviewed-by from anyone who can be trusted by the maintainer.


That is based upon the current situation - anyone can be a reviewer just because they want to be CCed and has no expectation to review the code.
Restricting R-B from a reviewer does not make sense to me.

Thank you
Yao, Jiewen



> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek
> Sent: Sunday, October 29, 2023 9:30 PM
> To: devel@edk2.groups.io; pedro.falcato@gmail.com; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Andrew Fish <afish@apple.com>; Leif Lindholm <quic_llindhol@quicinc.com>;
> Warkentin, Andrei <andrei.warkentin@intel.com>; West, Catharine
> <catharine.west@intel.com>; Bi, Dandan <dandan.bi@intel.com>; Daniel
> Schaefer <git@danielschaefer.me>; David Woodhouse <dwmw2@infradead.org>;
> De, Debkumar <debkumar.de@intel.com>; Dong, Eric <eric.dong@intel.com>;
> Jiang, Guomin <guomin.jiang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> James Bottomley <jejb@linux.ibm.com>; Wang, Jian J <jian.j.wang@intel.com>;
> Justen, Jordan L <jordan.l.justen@intel.com>; Julien Grall <julien@xen.org>;
> Peter Grehan <grehan@freebsd.org>; Zhang, Qi1 <qi1.zhang@intel.com>; Ng,
> Ray Han Lim <ray.han.lim.ng@intel.com>; Stefan Berger
> <stefanb@linux.ibm.com>; Hou, Wenxing <wenxing.hou@intel.com>; Lu, Xiaoyu1
> <xiaoyu1.lu@intel.com>
> Subject: Re: [edk2-devel] [Patch 1/1] Maintainers.txt: Update based on active
> community members
> 
> On 10/29/23 03:16, Pedro Falcato wrote:
> > On Sat, Oct 28, 2023 at 8:23 PM Michael D Kinney
> > <michael.d.kinney@intel.com> wrote:
> >>
> >> Over the past few months, all the of the Maintainers and
> >> Reviewers listed in Maintainers.txt have been contacted to make
> >> sure Maintainers.txt accurately represents the TianoCore
> >> community members that are actively participating in their
> >> roles.  Based on specific feedback, bounced emails, and no
> >> responses, updates have been made.
> >>
> >> * RISCV64: Daniel Schaefer replaced with Andrei Warkentin
> >> * ArmVirtPkg Xen has no remaining reviewers and review
> >>   responsibility defaults to ArmVirtPkg Maintainers/Reviewers.
> >> * ACPI modules related to S3 has no remaining reviewers and
> >>   review responsibility defaults to MdeModulePkg Maintainers/
> >>   Reviewers.
> >> * OVMF CSM modules has no remaining reviewers and review
> >>   responsibility defaults to OvmfPkg Maintainers/Reviewers.
> >> * Bounce: Chan Laura <laura.chan@intel.com>
> >> * Many smaller updates removing individuals that are no
> >>   longer involved or have replacement coverage.
> >
> > Mike,
> >
> > Thank you so much for doing this thankless task. Some comments:
> >
> >> diff --git a/Maintainers.txt b/Maintainers.txt
> >> index 3f40cdeb5554..2b03ccbe54aa 100644
> >> --- a/Maintainers.txt
> >> +++ b/Maintainers.txt
> >> @@ -93,7 +93,7 @@ M: Sami Mujawar <sami.mujawar@arm.com>
> [samimujawar]
> >>  RISCV64
> >>  F: */RiscV64/
> >>  M: Sunil V L <sunilvl@ventanamicro.com> [vlsunil]
> >> -R: Daniel Schaefer <git@danielschaefer.me> [JohnAZoidberg]
> >> +R: Andrei Warkentin <andrei.warkentin@intel.com> [andreiw]
> >>
> >>  LOONGARCH64
> >>  F: */LoongArch64/
> >> @@ -157,16 +157,6 @@ R: Leif Lindholm <quic_llindhol@quicinc.com>
> [leiflindholm]
> >>  R: Sami Mujawar <sami.mujawar@arm.com> [samimujawar]
> >>  R: Gerd Hoffmann <kraxel@redhat.com> [kraxel]
> >>
> >> -ArmVirtPkg: modules used on Xen
> >> -F: ArmVirtPkg/ArmVirtXen.*
> >> -F: ArmVirtPkg/Library/XenArmGenericTimerVirtCounterLib/
> >> -F: ArmVirtPkg/Library/XenVirtMemInfoLib/
> >> -F: ArmVirtPkg/PrePi/
> >> -F: ArmVirtPkg/XenAcpiPlatformDxe/
> >> -F: ArmVirtPkg/XenPlatformHasAcpiDtDxe/
> >> -F: ArmVirtPkg/XenioFdtDxe/
> >> -R: Julien Grall <julien@xen.org> [jgrall]
> >
> > ArmVirtPkg Xen modules seize to have a dedicated maintainer. Can the
> > generic ArmVirtPkg maintainers handle *more code* (particularly,
> > functionality that's not trivial to test, unless you actively use
> > Xen)?
> 
> An alternative to removing this entire section is to replace Julien's
> line with the following status line:
> 
> S: Orphan
> 
> The definition in Maintainers.txt is:
> 
>      Orphan:     No current maintainer [but maybe you could take the
>                  role as you write your new code].
> 
> I think this might be clearer for all three of: contributors, consumers,
> and existent maintainers.
> 
> - Contributors: An ArmVirtPkg maintainer may techincally merge your
> code, but you won't get substantive feedback
> 
> - Consumers: you can build and run this code, but if it breaks, you get
> to keep both parts
> 
> - Existent ArmVirtPkg maintainers: you can rest assured in the knowledge
> that you are not saddled with deep technical reviews for this subsystem
> that you can't even boot in your environment. You're only responsible
> for the technical act of merging patches.
> 
> >
> >>  BaseTools
> >>  F: BaseTools/
> >>  W: https://github.com/tianocore/tianocore.github.io/wiki/BaseTools
> >> @@ -187,8 +177,7 @@ F: CryptoPkg/
> >>  W: https://github.com/tianocore/tianocore.github.io/wiki/CryptoPkg
> >>  M: Jiewen Yao <jiewen.yao@intel.com> [jyao1]
> >>  M: Yi Li <yi1.li@intel.com> [liyi77]
> >> -R: Xiaoyu Lu <xiaoyu1.lu@intel.com> [xiaoyuxlu]
> >> -R: Guomin Jiang <guomin.jiang@intel.com> [guominjia]
> >> +R: Wenxing Hou <wenxing.hou@intel.com> [Wenxing-hou]
> >>
> >>  DynamicTablesPkg
> >>  F: DynamicTablesPkg/
> >> @@ -202,7 +191,6 @@ W:
> https://github.com/tianocore/tianocore.github.io/wiki/EmbeddedPkg
> >>  M: Leif Lindholm <quic_llindhol@quicinc.com> [leiflindholm]
> >>  M: Ard Biesheuvel <ardb+tianocore@kernel.org> [ardbiesheuvel]
> >>  M: Abner Chang <abner.chang@amd.com> [changab]
> >> -R: Daniel Schaefer <git@danielschaefer.me> [JohnAZoidberg]
> >>
> >>  EmulatorPkg
> >>  F: EmulatorPkg/
> >> @@ -228,7 +216,6 @@ F: FmpDevicePkg/
> >>  W: https://github.com/tianocore/tianocore.github.io/wiki/FmpDevicePkg
> >>  M: Liming Gao <gaoliming@byosoft.com.cn> [lgao4]
> >>  M: Michael D Kinney <michael.d.kinney@intel.com> [mdkinney]
> >> -R: Guomin Jiang <guomin.jiang@intel.com> [guominjia]
> >>  R: Wei6 Xu <wei6.xu@intel.com> [xuweiintel]
> >>
> >>  IntelFsp2Pkg
> >> @@ -237,7 +224,6 @@ W:
> https://github.com/tianocore/tianocore.github.io/wiki/IntelFsp2Pkg
> >>  M: Chasel Chiu <chasel.chiu@intel.com> [ChaselChiu]
> >>  M: Nate DeSimone <nathaniel.l.desimone@intel.com> [nate-desimone]
> >>  M: Duggapu Chinni B <chinni.b.duggapu@intel.com> [cbduggap]
> >> -M: Ray Han Lim Ng <ray.han.lim.ng@intel.com> [rayhanlimng]
> >>  R: Star Zeng <star.zeng@intel.com> [lzeng14]
> >>  R: Ted Kuo <ted.kuo@intel.com> [tedkuo1]
> >>  R: Ashraf Ali S <ashraf.ali.s@intel.com> [AshrafAliS]
> >> @@ -258,7 +244,6 @@ R: Susovan Mohapatra
> <susovan.mohapatra@intel.com> [susovanmohapatra]
> >>  MdeModulePkg
> >>  F: MdeModulePkg/
> >>  W: https://github.com/tianocore/tianocore.github.io/wiki/MdeModulePkg
> >> -M: Jian J Wang <jian.j.wang@intel.com> [jwang36]
> >>  M: Liming Gao <gaoliming@byosoft.com.cn> [lgao4]
> >
> > MdeModulePkg now only has a single maintainer (Liming, who also
> > handles a myriad of other tasks and packages)
> 
> This leads me to my main point: it may be time for edk2 to adopt a
> leaner contribution process.
> 
> We can insist on no patch going in without maintainer approval, but that
> -- i.e., maintainer authority -- only works as long as it goes hand in
> hand with maintainer responsibility: timely reviews. If the community
> cannot offer enough working hours for reviewing patches for a subsystem,
> then the requirements to contribute to that subsystem should be relaxed.
> The other alternative is that the subsystem goes into stasis, where it
> becomes effectively impossible to contribute to a subsystem.
> 
> (NB this "relaxation of contribution rules" is entirely orthogonal to
> using a mailing list vs. github pull requests. I still strongly prefer
> the mailing list.)
> 
> So maybe we could say, if there is no patch review for like 7 working
> days (approx. one and half calendar weeks), then the patch should be
> merged by default. Put differently, switch from "rejected by default" to
> "accepted by default".
> 
> By the way, I would like to assist with MdeModulePkg reviews. I'm not
> sure if I can *commit* to that, but right now, that is my intent. (As
> always, I see maintainership / reviewership as a service, not as a
> privilege.)
> 
> >>
> >>  MdeModulePkg: ACPI modules
> >> @@ -268,15 +253,6 @@ R: Zhiguang Liu <zhiguang.liu@intel.com>
> [LiuZhiguang001]
> >>  R: Dandan Bi <dandan.bi@intel.com> [dandanbi]
> >>  R: Liming Gao <gaoliming@byosoft.com.cn> [lgao4]
> >>
> >> -MdeModulePkg: ACPI modules related to S3
> >> -F: MdeModulePkg/*LockBox*/
> >> -F: MdeModulePkg/Include/*BootScript*.h
> >> -F: MdeModulePkg/Include/*LockBox*.h
> >> -F: MdeModulePkg/Include/*S3*.h
> >> -F: MdeModulePkg/Library/*S3*/
> >> -R: Hao A Wu <hao.a.wu@intel.com> [hwu25]
> >> -R: Eric Dong <eric.dong@intel.com> [ydong10]
> >> -
> >>  MdeModulePkg: BDS modules
> >>  F: MdeModulePkg/*BootManager*/
> >>  F: MdeModulePkg/Include/Library/UefiBootManagerLib.h
> 
> Same story could apply here -- we could orphan S3 stuff as well.
> 
> However, I can't deny I'm quite cranky at the thought of S3 breaking, at
> least in my trusty old configurations, so I'd certainly like to keep an
> eye on the S3 modules -- even if that only consisted of me
> regression-testing patches under OVMF (and not providing "expert
> feedback" on patch contents).
> 
> 
> >> @@ -326,7 +302,6 @@ F:
> MdeModulePkg/Library/DxeSecurityManagementLib/
> >>  F: MdeModulePkg/Universal/PCD/
> >>  F: MdeModulePkg/Universal/PlatformDriOverrideDxe/
> >>  F: MdeModulePkg/Universal/SecurityStubDxe/SecurityStub.c
> >> -R: Dandan Bi <dandan.bi@intel.com> [dandanbi]
> >>  R: Liming Gao <gaoliming@byosoft.com.cn> [lgao4]
> >
> > Down to one reviewer.
> 
> I'll try to assist whenever I can, wherever I notice something
> interesting -- I'm quite sure I'm going to be overwhelmed incredibly
> quickly, but at least I have that intent right now.
> 
> >
> >>
> >>  MdeModulePkg: Device and Peripheral modules
> >> @@ -346,12 +321,10 @@ F:
> MdeModulePkg/Include/Ppi/StorageSecurityCommand.h
> >>  F: MdeModulePkg/Include/Protocol/Ps2Policy.h
> >>  F: MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/
> >>  F: MdeModulePkg/Universal/PcatSingleSegmentPciCfg2Pei/
> >> -R: Hao A Wu <hao.a.wu@intel.com> [hwu25]
> >>  R: Ray Ni <ray.ni@intel.com> [niruiyu]
> >
> > Device and bus related code is down to one reviewer.
> >
> >>
> >>  MdeModulePkg: Disk modules
> >>  F: MdeModulePkg/Universal/Disk/
> >> -R: Hao A Wu <hao.a.wu@intel.com> [hwu25]
> >>  R: Ray Ni <ray.ni@intel.com> [niruiyu]
> >>  R: Zhichao Gao <zhichao.gao@intel.com> [ZhichaoGao]
> >>
> >> @@ -366,7 +339,6 @@ F:
> MdeModulePkg/Library/DisplayUpdateProgressLib*/
> >>  F: MdeModulePkg/Library/FmpAuthenticationLibNull/
> >>  F: MdeModulePkg/Universal/Esrt*/
> >>  R: Liming Gao <gaoliming@byosoft.com.cn> [lgao4]
> >> -R: Guomin Jiang <guomin.jiang@intel.com> [guominjia]
> >
> > One reviewer
> >
> >>
> >>  MdeModulePkg: HII and UI modules
> >>  F: MdeModulePkg/*FileExplorer*/
> >> @@ -383,7 +355,6 @@ F: MdeModulePkg/Universal/DisplayEngineDxe/
> >>  F: MdeModulePkg/Universal/DriverSampleDxe/
> >>  F: MdeModulePkg/Universal/SetupBrowserDxe/
> >>  R: Dandan Bi <dandan.bi@intel.com> [dandanbi]
> >> -R: Eric Dong <eric.dong@intel.com> [ydong10]
> >
> > One reviewer
> >>
> >>  MdeModulePkg: Management Mode (MM, SMM) modules
> >>  F: MdeModulePkg/*Smi*/
> >> @@ -395,10 +366,7 @@ R: Ray Ni <ray.ni@intel.com> [niruiyu]
> >>
> >>  MdeModulePkg: Pei Core
> >>  F: MdeModulePkg/Core/Pei/
> >> -R: Dandan Bi <dandan.bi@intel.com> [dandanbi]
> >>  R: Liming Gao <gaoliming@byosoft.com.cn> [lgao4]
> >> -R: Debkumar De <debkumar.de@intel.com> [dde01]
> >> -R: Catharine West <catharine.west@intel.com> [catharine-intl]
> >
> > The *PEI core* is now down to one reviewer.
> >
> >>
> 
> I've recently reviewed a PEI Core patch set! :)
> 
> >>  MdeModulePkg: Reset modules
> >>  F: MdeModulePkg/*Reset*/
> >> @@ -424,7 +392,6 @@ F: MdeModulePkg/Include/*/*Var*.h
> >>  F: MdeModulePkg/Include/Guid/SystemNvDataGuid.h
> >>  F: MdeModulePkg/Include/Protocol/SwapAddressRange.h
> >>  F: MdeModulePkg/Universal/FaultTolerantWrite*/
> >> -R: Hao A Wu <hao.a.wu@intel.com> [hwu25]
> >>  R: Liming Gao <gaoliming@byosoft.com.cn> [lgao4]
> >
> > ditto
> 
> This is under "MdeModulePkg: UEFI Variable modules".
> 
> Microsoft developers have contributed lots of UEFI variable-related
> stuff to edk2. Invite them to co-maintain / co-review?
> 
> >>
> >>  MdeModulePkg: Universal Payload definitions
> >> @@ -437,7 +404,6 @@ F: MdeModulePkg/Library/TraceHubDebugSysTLib/
> >>  F: MdeModulePkg/Include/Guid/TraceHubDebugInfoHob.h
> >>  M: Gua Guo <gua.guo@intel.com> [gguo11837463]
> >>  M: Prakashan Krishnadas Veliyathuparambil
> <krishnadas.veliyathuparambil.prakashan@intel.com> [kprakas2]
> >> -R: Chan Laura <laura.chan@intel.com> [lauracha]
> >>  R: K N Karthik <karthik.k.n@intel.com> [karthikkabbigere1]
> >>
> >>  MdeModulePkg: USB Network modules
> >> @@ -497,7 +463,6 @@ F: OvmfPkg/
> >>  W: http://www.tianocore.org/ovmf/
> >>  M: Ard Biesheuvel <ardb+tianocore@kernel.org> [ardbiesheuvel]
> >>  M: Jiewen Yao <jiewen.yao@intel.com> [jyao1]
> >> -R: Jordan Justen <jordan.l.justen@intel.com> [jljusten]
> >>  R: Gerd Hoffmann <kraxel@redhat.com> [kraxel]
> >>  S: Maintained
> >>
> >> @@ -513,7 +478,6 @@ F: OvmfPkg/Library/PlatformBootManagerLibBhyve/
> >>  F: OvmfPkg/Library/ResetSystemLib/BaseResetShutdownBhyve.c
> >>  F: OvmfPkg/Library/ResetSystemLib/BaseResetSystemLibBhyve.inf
> >>  R: Rebecca Cran <rebecca@bsdio.com> [bcran]
> >> -R: Peter Grehan <grehan@freebsd.org> [grehan-freebsd]
> >>  R: Corvin Köhne <corvink@freebsd.org> [corvink]
> >>
> >>  OvmfPkg: cloudhv-related modules
> >> @@ -528,10 +492,6 @@ F: OvmfPkg/Include/IndustryStandard/Microvm.h
> >>  F: OvmfPkg/Library/ResetSystemLib/*Microvm.*
> >>  R: Gerd Hoffmann <kraxel@redhat.com> [kraxel]
> >>
> >> -OvmfPkg: CSM modules
> >> -F: OvmfPkg/Csm/
> >> -R: David Woodhouse <dwmw2@infradead.org> [dwmw2]
> >
> > 0 people dedicated to OVMF CSM (although relatively low maintenance
> > overhead, from what it seems)
> 
> In my view, we should orphan the CSM now. Or maybe even better, mark it as
> 
>      Obsolete:   Old code. Something tagged obsolete generally means
>                  it has been replaced by a better system and you
>                  should be using that.
> 
> Mid-term, we should figure out a "feature deprecation process" for edk2,
> and then remove the CSM altogether. Other projects I'm somewhat familiar
> with have deprecation policies; they announce / document a subsystem
> deprecated in one release, and then a number of releases later, the
> subsystem is removed completely. This gives users notice ahead of time,
> and lets them migrate to different solutions gradually.
> 
> Lots of edk2 code have been removed already (Itanium support, Intel
> Framework stuff, etc); we didn't observe any deprecation policy back
> then. I don't know if there was any backlash from that. I'd be OK with
> removing the CSM at once (well, not in edk2-stable202311, but in the
> release after), but that might not be perceived as overly polite.
> 
> >> -
> >>  OvmfPkg: Confidential Computing
> >>  F: OvmfPkg/AmdSev/
> >>  F: OvmfPkg/AmdSevDxe/
> >> @@ -545,7 +505,6 @@ F: OvmfPkg/PlatformPei/AmdSev.c
> >>  F: OvmfPkg/ResetVector/
> >>  F: OvmfPkg/Sec/
> >>  R: Erdem Aktas <erdemaktas@google.com> [ruleof2]
> >> -R: James Bottomley <jejb@linux.ibm.com> [jejb]
> >>  R: Jiewen Yao <jiewen.yao@intel.com> [jyao1]
> >>  R: Min Xu <min.m.xu@intel.com> [mxu9]
> >>  R: Tom Lendacky <thomas.lendacky@amd.com> [tlendacky]
> 
> It's good for the project that CoCo has several reviewers still.
> 
> (It's one of those areas that I categorically refuse to look at.
> 
> I might make an exception for base SEV, but SEV-ES is quite unlikely,
> and SEV-SNP and TDX are out of question for me.)
> 
> >> @@ -568,7 +527,6 @@ F: OvmfPkg/Library/Tcg2PhysicalPresenceLib*/
> >>  F: OvmfPkg/PlatformPei/ClearCache.c
> >>  F: OvmfPkg/Tcg/
> >>  R: Marc-André Lureau <marcandre.lureau@redhat.com> [elmarco]
> >> -R: Stefan Berger <stefanb@linux.ibm.com> [stefanberger]
> >
> > One reviewer
> 
> I'll attempt to help here (TCG/TPM2) if necessary; even if that's going
> to boil down to summon more knowledgeable folks from Red Hat :)
> 
> >>
> >>  OvmfPkg: Xen-related modules
> >>  F: OvmfPkg/Include/Guid/XenBusRootDevice.h
> >> @@ -597,7 +555,6 @@ F: OvmfPkg/XenPlatformPei/
> >>  F: OvmfPkg/XenPvBlkDxe/
> >>  F: OvmfPkg/XenResetVector/
> >>  R: Anthony Perard <anthony.perard@citrix.com> [tperard]
> >> -R: Julien Grall <julien@xen.org> [jgrall]
> >
> > One reviewer
> 
> Not necessarily a bad thing, my impression is that OVMF Xen has seen
> very little churn. At least it's not unmaintained.
> 
> >>
> >>  OvmfPkg: RISC-V Qemu Virt Platform
> >>  F: OvmfPkg/RiscVVirt
> >> @@ -627,7 +584,6 @@ SecurityPkg
> >>  F: SecurityPkg/
> >>  W: https://github.com/tianocore/tianocore.github.io/wiki/SecurityPkg
> >>  M: Jiewen Yao <jiewen.yao@intel.com> [jyao1]
> >> -M: Jian J Wang <jian.j.wang@intel.com> [jwang36]
> >>
> >>  SecurityPkg: Secure boot related modules
> >>  F: SecurityPkg/Library/DxeImageVerificationLib/
> >> @@ -637,7 +593,6 @@ R: Min Xu <min.m.xu@intel.com> [mxu9]
> >>
> >>  SecurityPkg: Tcg related modules
> >>  F: SecurityPkg/Tcg/
> >> -R: Qi Zhang <qi1.zhang@intel.com> [qizhangz]
> >>  R: Rahul Kumar <rahul1.kumar@intel.com> [rahul1-kumar]
> >
> > ditto
> 
> This still falls under Jiewen's maintainership of SecurityPkg, so I
> don't perceive it as very risky.
> 
> >>
> >>  ShellPkg
> >> @@ -648,12 +603,10 @@ M: Zhichao Gao <zhichao.gao@intel.com>
> [ZhichaoGao]
> >>  SignedCapsulePkg
> >>  F: SignedCapsulePkg/
> >>  W: https://github.com/tianocore/tianocore.github.io/wiki/SignedCapsulePkg
> >> -M: Jian J Wang <jian.j.wang@intel.com> [jwang36]
> >
> > Unmaintained
> 
> Probably best to mark it as orphaned then!
> 
> >
> >>
> >>  SourceLevelDebugPkg
> >>  F: SourceLevelDebugPkg/
> >>  W:
> https://github.com/tianocore/tianocore.github.io/wiki/SourceLevelDebugPkg
> >> -M: Hao A Wu <hao.a.wu@intel.com> [hwu25]
> >
> > Unmaintained
> >>
> >>  StandaloneMmPkg
> >>  F: StandaloneMmPkg/
> 
> I'd orphan this one as well. For one, I've never gotten
> SOURCE_DEBUG_ENABLE to work in OVMF.
> 
> (I'd not go as far as removing it, I'm sure this module has many
> downstream users!)
> 
> 
> >> @@ -664,7 +617,6 @@ M: Ray Ni <ray.ni@intel.com> [niruiyu]
> >>  UefiCpuPkg
> >>  F: UefiCpuPkg/
> >>  W: https://github.com/tianocore/tianocore.github.io/wiki/UefiCpuPkg
> >> -M: Eric Dong <eric.dong@intel.com> [ydong10]
> >>  M: Ray Ni <ray.ni@intel.com> [niruiyu]
> >>  R: Rahul Kumar <rahul1.kumar@intel.com> [rahul1-kumar]
> >>  R: Gerd Hoffmann <kraxel@redhat.com> [kraxel]
> >> @@ -672,7 +624,6 @@ R: Gerd Hoffmann <kraxel@redhat.com> [kraxel]
> >>  UefiCpuPkg: Sec related modules
> >>  F: UefiCpuPkg/SecCore/
> >>  F: UefiCpuPkg/ResetVector/
> >> -R: Debkumar De <debkumar.de@intel.com> [dde01]
> >>  R: Catharine West <catharine.west@intel.com> [catharine-intl]
> >
> > One reviewer.
> 
> Not necessarily alarming IMO, UefiCpuPkg in general is not neglected
> (Gerd is listed, and I would like to keep an eye on it too). So I'd
> rather phrase this one as "we even have a dedicated reviewer for
> 'UefiCpuPkg: Sec'!" :)
> 
> >
> > Some brief LoC (taking into account code, blank lines and comments)
> > stats over some of the affected packages/modules:
> > SignedCapsulePkg - 6,836 LoC
> > SourceLevelDebugPkg - 15,208 LoC
> > MdeModulePkg - 616,591 LoC (!!)
> >    Bus/ -                216,268 LoC (!!!)
> 
> Yep, these two are heavy-weights.
> 
> > (HII and UI was tough to actually measure, but I'm relatively sure
> > it's 100,000+ LoC!)
> 
> HII is unfortunately terribly difficult. The documentation is very
> lacking IMO (in the spec). I tried to read Tim Lewis's blog posts on it:
> 
>   https://uefi.blogspot.com/search/label/UEFI%20HII
> 
> but I didn't get far. It feels like one of the most over-engineered (or
> at least most complex) parts of UEFI / edk2. I once authored
> OvmfPkg/PlatformDxe, because Jordan really wanted me to; ever since I've
> been steering as clear of it as I could :) At least Dandan continues as
> a designated reviewer for HII!
> 
> >   Core/Pei  - 11,985 LoC
> > SecurityPkg/Tcg - 26,275 LoC
> >
> > (sidenote: It'd be interesting to see the numbers from a personnel PoV
> > - Person X is responsible for N lines of code, etc)
> > It seems obvious (as a result of your great work!) that lots of people
> > really are stretched incredibly thin.
> >
> > Taking everything into account, I have two questions:
> > 1) Should we go through these changes (that effectively reflect
> > reality, that much I understand) and see what needs to be cut from
> > EDK2 (i.e do we have an overabundance of features)?
> 
> I'd say "yes". To reiterate,
> 
> - I'd propose explicitly marking orphaned subsystems as such, rather
> than merging them silently into parent subsystems,
> 
> - certainly removing some subsystems, but for that, a "staged"
> deprecation policy would be most polite.
> 
> > 2) What's the call for action here? Should people submit themselves as
> > new reviewers/maintainers of poorly maintained/reviewed code?
> Themselves and each other, yes.
> 
> I'll re-raise my point about relaxing the contribution conditions too --
> given this state, I'd propose a "merge by default" approach, with a
> reasonable timeout.
> 
> Laszlo
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110285): https://edk2.groups.io/g/devel/message/110285
Mute This Topic: https://groups.io/mt/102245264/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



  parent reply	other threads:[~2023-10-30  2:54 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-28 19:23 [edk2-devel] [Patch 1/1] Maintainers.txt: Update based on active community members Michael D Kinney
2023-10-29  2:16 ` Pedro Falcato
2023-10-29  8:05   ` Yao, Jiewen
2023-10-29 13:48     ` Laszlo Ersek
2023-10-29 14:09       ` Laszlo Ersek
2023-10-29 15:42       ` Yao, Jiewen
2023-10-29 16:01         ` James Bottomley
2023-10-29 16:25           ` Yao, Jiewen
2023-10-29 17:22             ` Michael D Kinney
2023-10-30  2:40               ` Yao, Jiewen
2023-10-30 10:44                 ` Laszlo Ersek
2023-10-29 18:29             ` Pedro Falcato
2023-10-29 13:30   ` Laszlo Ersek
2023-10-29 19:01     ` Pedro Falcato
2023-10-30 11:25       ` Laszlo Ersek
2023-10-30  2:54     ` Yao, Jiewen [this message]
2023-10-30  5:31       ` Michael D Kinney
2023-10-30 11:29         ` Laszlo Ersek
2023-10-30 22:18           ` Michael D Kinney
2023-10-31 10:16             ` Laszlo Ersek
2023-10-30  7:38   ` Ng, Ray Han Lim
2023-10-29 21:58 ` Stefan Berger
2023-10-30  4:51 ` Peter Grehan
2023-10-30  7:35 ` Wu, Hao A
2023-10-30 10:51 ` Julien Grall
2023-10-31  4:08 ` Andrei Warkentin
2023-10-31  6:25 ` Jordan Justen
2023-10-31 10:24 ` Laszlo Ersek
2023-11-05  1:22   ` Michael D Kinney
2023-11-05 10:28     ` Laszlo Ersek
2023-10-31 12:27 ` Leif Lindholm
2023-11-04 23:25   ` Michael D Kinney

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=MW4PR11MB5872D65D4E889737C49C98538CA1A@MW4PR11MB5872.namprd11.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox