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

["Pedro Falcato" <pedro.falcato@gmail.com>]

On Sun, Oct 29, 2023 at 4:25 PM Yao, Jiewen <jiewen.yao@intel.com> wrote:
>
> 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.

The Linux development process, as I understand it (it may be a bit
imprecise, AFAIK lots of it isn't written):

Maintainers are the primary caretakers of the code. They'll review and
merge patches (in linux, they usually add their own Signed-off-by,
they don't do Reviewed-by). Sometimes, they'll post a patch on the
mailing list, and if there's no poor feedback, they just merge it to
their trees, unilaterally (for Linux, Linus usually pulls from
maintainer trees, and if he doesn't like something he *will* tell
you).

Reviewers are people that want to be CC'd and want to review patches,
but they're not expected to always do so. There's of course an
expectation that reviewers are relatively competent in the area
they're reviewing.
There's an expectation that you will help out and participate in code
review from time to time.

As an example:

SLAB ALLOCATOR
M: Christoph Lameter <cl@linux.com>
M: Pekka Enberg <penberg@kernel.org>
M: David Rientjes <rientjes@google.com>
M: Joonsoo Kim <iamjoonsoo.kim@lge.com>
M: Andrew Morton <akpm@linux-foundation.org>
M: Vlastimil Babka <vbabka@suse.cz>
R: Roman Gushchin <roman.gushchin@linux.dev>
R: Hyeonggon Yoo <42.hyeyoo@gmail.com>
L: linux-mm@kvack.org
S: Maintained
T: git git://git.kernel.org/pub/scm/linux/kernel/git/vbabka/slab.git
F: include/linux/sl?b*.h
F: mm/sl?b*

For a patch for mm/slab.c, you CC all M's and R's. Maintainers are the
one with the 'power' to merge it, and should give you feedback.
Reviewers sometimes help out (but are secondary in the patch review
role), but they cannot merge patches.
You only merge a patch if there's an understanding that most people
and stakeholders are ok with it; for example, you may want feedback
from people that are not M's and R's. If everyone is ok with your
patch, a maintainer will apply it (in this case, it's vbabka's tree so
he usually takes care of it), and it will eventually trickle up to
Linus (who manages the 'master' git tree) who gives the final seal of
approval when pulling changes.

For a smaller example, we can look at EFI, which has a sole maintainer
(Ard) and no reviewers; this is ok (EFI is a lot less central to the
kernel than SLAB, there are a lot less patches), but stakeholders in
the various changes should still test and review.

This is a nice rough description of the whole development process:
https://docs.kernel.org/process/2.Process.html
Note that EDK2 is considerably smaller than the kernel in scope and
patch volume, so it probably doesn't make much sense to be as
distributed as Linux.

PS: It's worth noting that the Linux equivalent to GetMaintainers.py
takes into account the git blame of the files you're touching, terms
you mention in the commit message; that is, it will automatically pick
up people that have touched the code before or are responsible for
features you're interacting with.

-- 
Pedro


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