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

[James Bottomley]

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/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-