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

["Yao, Jiewen" <jiewen.yao@intel.com>]

> 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 <lersek@redhat.com>
> Sent: Sunday, October 29, 2023 9:48 PM
> To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.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>;
> James Bottomley <jejb@linux.ibm.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
> 
> 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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-