Re: [edk2-devel] [Patch 1/1] Maintainers.txt: Update based on active community members

["Yao, Jiewen" <jiewen.yao@intel.com>]

Thanks Mike. I am reading the WIKI page.


> and/or provides testing or regression testing for the Package (or some modules thereof), in certain platforms and environments.

[Jiewen] Are we expecting Reviewer to provide testing or regression testing for the package?
Is that what the reviewer *commits* to do?
For example, Maintainer may ask the reviewer to do some testing, right?


> Reviewer is responsible for timely responses on emails addressed to them (preferably less than calendar week).

[Jiewen]
Is that what the reviewer *commits* to do?
For example, Maintainer may ask the reviewer to provide feedback, right?


Those are more than just CCed.


Thank you
Yao, Jiewen


> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Monday, October 30, 2023 1:23 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>; jejb@linux.ibm.com; Laszlo Ersek
> <lersek@redhat.com>; devel@edk2.groups.io; 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>;
> 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
> 
> This is the Wiki page where TianoCore documents the TianoCore community
> member roles.
> 
> https://github.com/tianocore/tianocore.github.io/wiki/TianoCore-Who-we-are
> 
> We can update/edit as needed to accurately reflect what all the Maintainers
> and Reviewers agree are their roles and responsibilities as assigned in
> Maintainers.txt.
> 
> Thanks,
> 
> Mike
> 
> 
> > -----Original Message-----
> > From: Yao, Jiewen <jiewen.yao@intel.com>
> > Sent: Sunday, October 29, 2023 9:26 AM
> > To: jejb@linux.ibm.com; Laszlo Ersek <lersek@redhat.com>;
> > 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>; 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
> >
> > OK. Maintainer should do code review. I have no doubt on that.
> >
> > My confusion is about "reviewer" role. What is criteria and what is
> > responsibility?
> >
> > Are you saying that "reviewer" just means that someone raised the hand
> > and said: "I want to be notified", and there is no expectation that
> > he/she would review the patch?
> >
> > I would like to understand more on how that works and what that means.
> > Would you please give a URL for the reviewer definition in Linux
> > Kernel?
> >
> > Thank you
> > Yao, Jiewen
> >
> >
> >
> > > -----Original Message-----
> > > From: James Bottomley <jejb@linux.ibm.com>
> > > Sent: Monday, October 30, 2023 12:02 AM
> > > To: Yao, Jiewen <jiewen.yao@intel.com>; Laszlo Ersek
> > <lersek@redhat.com>;
> > > 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>;
> > > 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 Sun, 2023-10-29 at 15:42 +0000, Yao, Jiewen wrote:
> > > > > I'd say that's pretty close. A reviewer role is a request for
> > > > > keeping
> > > > > the reviewer in the loop.
> > > >
> > > > [Jiewen] I am disappointed on that.
> > > > To me, that is NOT a real reviewer. See below description on what
> > is
> > > > "code review".
> > > > https://google.github.io/eng-practices/review/
> > > > https://about.gitlab.com/topics/version-control/what-is-code-
> > review/
> > >
> > > Well, that's what someone's view of what a patch review should
> > consist
> > > of, not what a reviewer's role in MAINTAINERS should be.
> > >
> > > In general, you really don't want to force people to review patches,
> > > because you'd like a reviewer to be familiar with the area and
> > > comfortable with the patch.  So are you saying that anyone listed as
> > a
> > > reviewer in a particular area should be capable of reviewing any
> > patch?
> > > and further that they should be expected to review every patch?
> > > Because that's definitely not what the R role in the Linux Kernel
> > would
> > > mean.
> > >
> > > I know that's not what happened to me in Confidential Computing,
> > > because I had a very specific area around SEV and SEV-ES secret
> > > injection and really had no familiarity at all with say the memory
> > > acceptance patches.
> > >
> > > > Our definition seems more like *a notification receiver*, instead
> > of
> > > > a real code reviewer. I would say, it is a very misleading
> > > > definition.
> > >
> > > Actually, I wouldn't, but then I'm more coming from a Linux Kernel
> > > background.  To us, the reviewer list is simply a list of people git
> > > blame might not find who might have the expertise to review the
> > patch
> > > but on whom there would be no expectation that they would review the
> > > patch.
> > >
> > > James



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110284): https://edk2.groups.io/g/devel/message/110284
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]
-=-=-=-=-=-=-=-=-=-=-=-