public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Michael D Kinney" <michael.d.kinney@intel.com>
To: "Yao, Jiewen" <jiewen.yao@intel.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>,
	"lersek@redhat.com" <lersek@redhat.com>,
	"pedro.falcato@gmail.com" <pedro.falcato@gmail.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>,
	"Kinney, Michael D" <michael.d.kinney@intel.com>
Subject: Re: [edk2-devel] [Patch 1/1] Maintainers.txt: Update based on active community members
Date: Mon, 30 Oct 2023 05:31:10 +0000	[thread overview]
Message-ID: <CO1PR11MB4929385236449CB00A822DC3D2A1A@CO1PR11MB4929.namprd11.prod.outlook.com> (raw)
In-Reply-To: <MW4PR11MB5872D65D4E889737C49C98538CA1A@MW4PR11MB5872.namprd11.prod.outlook.com>

There is a very good discussion here on the roles and responsibility
and potential suggestions for changes to the Wiki page that document
those roles and responsibilities.

May I suggest that someone start a new thread that discusses
the proposed changes to the Wiki page and leave this thread for the
review of the changes to Maintainers.txt?

Thanks,

Mike

> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: Sunday, October 29, 2023 7:54 PM
> To: devel@edk2.groups.io; lersek@redhat.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
> 
> > 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/SourceLevelDebug
> Pkg
> > >> -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 (#110288): https://edk2.groups.io/g/devel/message/110288
Mute This Topic: https://groups.io/mt/102245264/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



  reply	other threads:[~2023-10-30  5:31 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
2023-10-30  5:31       ` Michael D Kinney [this message]
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=CO1PR11MB4929385236449CB00A822DC3D2A1A@CO1PR11MB4929.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