On Sun, Oct 29, 2023 at 4:25 PM Yao, Jiewen <jiewen....@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 <c...@linux.com> M: Pekka Enberg <penb...@kernel.org> M: David Rientjes <rient...@google.com> M: Joonsoo Kim <iamjoonsoo....@lge.com> M: Andrew Morton <a...@linux-foundation.org> M: Vlastimil Babka <vba...@suse.cz> R: Roman Gushchin <roman.gushc...@linux.dev> R: Hyeonggon Yoo <42.hye...@gmail.com> L: linux...@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/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
