From: "James Bottomley" <jejb@linux.ibm.com>
To: "Yao, Jiewen" <jiewen.yao@intel.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 12:01:34 -0400 [thread overview]
Message-ID: <c9ba15d43451384dfb72646fd3167d600e67aab7.camel@linux.ibm.com> (raw)
In-Reply-To: <MW4PR11MB58727114FDDF155721B115AE8CA2A@MW4PR11MB5872.namprd11.prod.outlook.com>
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 (#110270): https://edk2.groups.io/g/devel/message/110270
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]
-=-=-=-=-=-=-=-=-=-=-=-
next prev parent reply other threads:[~2023-10-29 16:01 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 [this message]
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
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=c9ba15d43451384dfb72646fd3167d600e67aab7.camel@linux.ibm.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