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

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

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 (#110271): https://edk2.groups.io/g/devel/message/110271
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]
-=-=-=-=-=-=-=-=-=-=-=-