public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Yao, Jiewen" <jiewen.yao@intel.com>
To: "jejb@linux.ibm.com" <jejb@linux.ibm.com>,
	Laszlo Ersek <lersek@redhat.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>,
	"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>,
	"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: Sun, 29 Oct 2023 16:25:35 +0000	[thread overview]
Message-ID: <MW4PR11MB58725A66FB70532DC5BB99468CA2A@MW4PR11MB5872.namprd11.prod.outlook.com> (raw)
In-Reply-To: <c9ba15d43451384dfb72646fd3167d600e67aab7.camel@linux.ibm.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]
-=-=-=-=-=-=-=-=-=-=-=-



  reply	other threads:[~2023-10-29 16:25 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 [this message]
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
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=MW4PR11MB58725A66FB70532DC5BB99468CA2A@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