> 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/ Our definition seems more like *a notification receiver*, instead of a real code reviewer. I would say, it is a very misleading definition. Thank you Yao, Jiewen > -----Original Message----- > From: Laszlo Ersek <ler...@redhat.com> > Sent: Sunday, October 29, 2023 9:48 PM > To: devel@edk2.groups.io; Yao, Jiewen <jiewen....@intel.com>; > pedro.falc...@gmail.com; Kinney, Michael D <michael.d.kin...@intel.com> > Cc: Andrew Fish <af...@apple.com>; Leif Lindholm <quic_llind...@quicinc.com>; > Warkentin, Andrei <andrei.warken...@intel.com>; West, Catharine > <catharine.w...@intel.com>; Bi, Dandan <dandan...@intel.com>; Daniel > Schaefer <g...@danielschaefer.me>; David Woodhouse <dw...@infradead.org>; > De, Debkumar <debkumar...@intel.com>; Dong, Eric <eric.d...@intel.com>; > Jiang, Guomin <guomin.ji...@intel.com>; Wu, Hao A <hao.a...@intel.com>; > James Bottomley <j...@linux.ibm.com>; Wang, Jian J <jian.j.w...@intel.com>; > Justen, Jordan L <jordan.l.jus...@intel.com>; Julien Grall <jul...@xen.org>; > Peter Grehan <gre...@freebsd.org>; Zhang, Qi1 <qi1.zh...@intel.com>; Ng, > Ray Han Lim <ray.han.lim...@intel.com>; Stefan Berger > <stef...@linux.ibm.com>; Hou, Wenxing <wenxing....@intel.com>; Lu, Xiaoyu1 > <xiaoyu1...@intel.com> > Subject: Re: [edk2-devel] [Patch 1/1] Maintainers.txt: Update based on active > community members > > On 10/29/23 09:05, Yao, Jiewen wrote: > > Those are great questions. I also would like to understand: > > > > 1) What is definition of "actively participating in their roles"? > > Here are the definitions of Maintainer and Reviewer, from > "Maintainers.txt": > > M: Package Maintainer: Cc address for patches and questions. Responsible > for reviewing and pushing package changes to source control. > R: Package Reviewer: Cc address for patches and questions. Reviewers help > maintainers review code, but don't have push access. A designated Package > Reviewer is reasonably familiar with the Package (or some modules > thereof), and/or provides testing or regression testing for the Package > (or some modules thereof), in certain platforms and environments. > > > Is there any enforcement or just volunteer work? > > I see the Maintainer role as a service to the community, with some > benefits granted in return. The "service" part should be clear. The > benefit is that you are kept in the loop, and sometimes (when you must) > you *can* say "no". (According to some seasoned reviewers, the one real > power of a maintainer -- not to be abused! -- is "saying no".) A > maintainer that's present helps set the focus, keep regressions out, > gives advice when needed, and so on. > > Enforcement would be nice (haha), but it never works. You can't force > people to help, especially if their dayjob instructions oppose their > upstream community responsibilities. That's fine; in such cases my > request is always: if you can't help, then at least don't get in the > way, step down. Don't block people from doing their work by having them > wait for your feedback. > > So volunteer work is fine, but as soon as the position grows "fangs" (= > a capacity to make others wait), then it becomes a promise, a > responsibility. > > > > > 2) What is role and *responsibility* of Reviewer role? Is it > > documented somewhere? > > Per my observation, some assigned reviewers have never reviewed any > > patch in history or provided valuable feedback. To me, reviewer role > > seems more like a notification instead of really review something. Is > > that our purpose? > > I'd say that's pretty close. A reviewer role is a request for keeping > the reviewer in the loop. Maintainers tend to appreciate that, because a > long-term reviewer may provide good insights, test results, and so on. > Trust is super important; a maintainer may push a patch based solely on > a reviewer's positive feedback, due to the latter's experience. > > > While Laszlo contributed a lots in Tianocore community, he is really a > > good "reviewer", although he has no such title. > > Thanks for the acknowledgement, I appreciate it! > > I don't like to hoard titles. I'm sure titles are good for one's career, > but I always see the *promise* (the responsibility) to the community, > first and foremost, that a title encapsulates. It weighs heavily on me. > I loathe disappointing people. For me, not to bear a title is better > than to bear it and not to deliver on it / not to live up to it. > > Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#110269): https://edk2.groups.io/g/devel/message/110269 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] -=-=-=-=-=-=-=-=-=-=-=-
