Re: 回复: [EXTERNAL] RE: [edk2-announce] 回复: [edk2-devel] Tianocore community page on who we are - please review

["Laszlo Ersek" <lersek@redhat.com>]

On 09/30/20 11:25, gaoliming wrote:
> Jiewen:
>   Frankly speaking, I don't know this rule that the patch needs to get review or ack from the maintainer. When the reviewer name is formally added into maintainers.txt, I think the maintainer has delegated the approval work to reviewers. So, I think that the reviewer takes the same role to the maintainer except for the patch merge.

As far as I remember, the intent to designate reviewers in the
Maintainers.txt file was (a) to highlight people that consistently
review patches for a subsystem *without* having push rights, (b) to make
sure that patch submitters would CC those people on their postings
*up-font*. Participation of reviewers does not substitute 100% for
maintainer action.

Thanks
Laszlo

> 
> Thanks
> Liming
>> -----邮件原件-----
>> 发件人: bounce+27952+65748+4905953+8761045@groups.io
>> <bounce+27952+65748+4905953+8761045@groups.io> 代表 Yao, Jiewen
>> 发送时间: 2020年9月30日 10:12
>> 收件人: Leif Lindholm <leif@nuviainc.com>
>> 抄送: gaoliming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io;
>> Guptha, Soumya K <soumya.k.guptha@intel.com>;
>> announce@edk2.groups.io; lersek@redhat.com; Kinney, Michael D
>> <michael.d.kinney@intel.com>; 'Andrew Fish' <afish@apple.com>
>> 主题: Re: [EXTERNAL] RE: [edk2-announce] 回复: [edk2-devel] Tianocore
>> community page on who we are - please review
>>
>> Hi Leif and Liming
>> I have double checked with Mike Kinney on the role and responsibility of
>> reviewers.
>> Mike and I reach the consensus below (a short version, detail will be added to
>> the wiki page later):
>>
>> 1) Maintainers are the ONLY ones who can approve a patch.
>> 2) Reviewers CANNOT approve the patch. (*)
>> 3) A maintainer CANNOT approve his/her own patch.
>> 4) Maintainers MAY delegate the approval work to reviewers.
>>
>> So the final state of the commit message as a minimum must be either:
>>              Reviewed-by: <Package Maintainer>
>> Or:
>>              Acked-by: <Package Maintainer>
>>              Reviewed-by: <Package Reviewer>
>>
>> All in all, I don’t think it is correct to say "Reviewers can approve the patch.
>> The only additional work from maintainers is to check in the patch".
>>
>> Please let us know if you have different thought.
>>
>> Thank you
>> Yao Jiewen
>>
>>> -----Original Message-----
>>> From: Leif Lindholm <leif@nuviainc.com>
>>> Sent: Monday, September 28, 2020 8:02 PM
>>> To: Yao, Jiewen <jiewen.yao@intel.com>
>>> Cc: gaoliming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io;
>> Guptha,
>>> Soumya K <soumya.k.guptha@intel.com>; announce@edk2.groups.io;
>>> lersek@redhat.com; Kinney, Michael D <michael.d.kinney@intel.com>;
>> 'Andrew
>>> Fish' <afish@apple.com>
>>> Subject: Re: [EXTERNAL] RE: [edk2-announce] 回复: [edk2-devel]
>> Tianocore
>>> community page on who we are - please review
>>>
>>> Hi Jiewen,
>>>
>>> On Sun, Sep 27, 2020 at 03:25:24 +0000, Yao, Jiewen wrote:
>>>> Thanks Liming.
>>>>
>>>> It seems I have some misunderstanding here.
>>>>
>>>> According to current process, I feel that only maintainer has right to
>>> *approve* the patch.
>>>> The reviewer cannot approve the patch.
>>>> Do you mean the reviewer can also approve the patch?
>>>
>>> My view is that a reviewer has a right to "approve" a patch, but they
>>> do not have access to actually push the patch. A maintainer is needed
>>> for that. In instances where a designated maintainer is unavaliable to
>>> do so, another maintainer would be permitted to push the patch.
>>>
>>> In instances where the designated maintainer disagrees with the
>>> reviewer, the patch should not be pushed. However, the same should be
>>> true for a patch where two designated maintainers or two designated
>>> reviewers disagree.
>>>
>>> Best Regards,
>>>
>>> Leif
>>>
>>>> According to
>> https://github.com/tianocore/tianocore.github.io/wiki/Who-we-
>>> are#role-of-a-reviewer, I don’t think "Reviewer takes role 1~4.". (I am
>> confused
>>> here ... So please do correct me if I am wrong.)
>>>> =================
>>>> Role of a Reviewer
>>>> Reviewers help maintainers review code, but don't have push access.
>>>>
>>>> A designated Package Reviewer:
>>>>
>>>> shall be reasonably familiar with the Package (or some modules thereof)
>>>>
>>>> will be copied on the patch discussions,
>>>>
>>>> and/or provides testing or regression testing for the Package (or some
>>> modules thereof), in certain platforms and environments.
>>>> ================
>>>>
>>>> Thank you
>>>> Yao Jiewen
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: announce@edk2.groups.io <announce@edk2.groups.io> On
>> Behalf Of
>>>>> gaoliming
>>>>> Sent: Sunday, September 27, 2020 10:33 AM
>>>>> To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>;
>> Guptha,
>>>>> Soumya K <soumya.k.guptha@intel.com>; announce@edk2.groups.io
>>>>> Cc: lersek@redhat.com; 'Leif Lindholm (Nuvia address)'
>> <leif@nuviainc.com>;
>>>>> Kinney, Michael D <michael.d.kinney@intel.com>; 'Andrew Fish'
>>>>> <afish@apple.com>
>>>>> Subject: [edk2-announce] 回复: [edk2-devel] Tianocore community
>> page on
>>>>> who we are - please review
>>>>>
>>>>> Jiewen:
>>>>>
>>>>>  Now, we have reviewer and maintainer role. Reviewer takes role 1~4.
>>>>> Maintainer takes role 1~7. If the people know edk2 process well, they
>> mostly
>>>>> know edk2 one or more packages (modules). So, they can take
>> Maintainer
>>> role.
>>>>> If the people only focus on the technical review, they can take reviewer
>>>>> role. I would suggest there is at lease one Maintainer for each package.
>>>>> There are more reviewers for each package.
>>>>>
>>>>>
>>>>>
>>>>> Soumya:
>>>>>
>>>>>  Here are my comments.
>>>>>
>>>>>   Guidelines for a Maintainer. Never let a pending request get older
>> than a
>>>>> calendar week. This requirement is too strict to the maintainer or
>> reviewer.
>>>>> The maintainer or reviewer should try to give the response in one week.
>> But,
>>>>> they may not fully review one patch set in one week, es for the feature
>> or
>>>>> the complex change.
>>>>>
>>>>> Role of a Contributor/developer. We need to highlight the role &
>>>>> responsibility for the incompatible change. If the contributor proposes
>> the
>>>>> incompatible change, he needs to coordinate with the impacted platform
>>>>> maintainer and make the agreement who will follow up to update the
>>> impacted
>>>>> platforms before he requests to merge his patch set. The impacted
>> platforms
>>>>> include all ones in Edk2 and Edk2Platforms.
>>>>>
>>>>>
>>>>>
>>>>> Last, this page also needs to include release maintainer Definition and
>>>>> Role. The release maintainer is to create the quarterly stable tag. He
>> takes
>>>>> the role to collect the feature planning for each stable tag, schedule the
>>>>> release date, and create the stable tag with the release notes on tag
>> page.
>>>>> He will also send the announcement of soft feature freeze, hard feature
>>>>> freeze and the stable tag completement to edk2 community.
>>>>>
>>>>>
>>>>>
>>>>> Thanks
>>>>>
>>>>> Liming
>>>>>
>>>>> 发件人: bounce+27952+65655+4905953+8761045@groups.io
>>>>> <bounce+27952+65655+4905953+8761045@groups.io> 代表 Yao,
>> Jiewen
>>>>> 发送时间: 2020年9月26日 13:33
>>>>> 收件人: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>;
>>> Guptha,
>>>>> Soumya K <soumya.k.guptha@intel.com>; announce@edk2.groups.io
>>>>> 主题: Re: [edk2-devel] Tianocore community page on who we are -
>> please
>>>>> review
>>>>>
>>>>>
>>>>>
>>>>> Some other thought is about maintainer’s role definition:
>>>>>
>>>>>
>>>>>
>>>>> The role of a maintainer is to:
>>>>>
>>>>> 1.	Maintainer assignments to packages and source file name patterns
>> are
>>>>> provided in the "
>>>>> <https://github.com/tianocore/edk2/blob/master/Maintainers.txt>
>>> Maintainers.
>>>>> txt" file.
>>>>> 2.	Subscribe to the "edk2-bugs" mailing list
>>>>> <https://edk2.groups.io/g/bugs> https://edk2.groups.io/g/bugs, which
>>>>> propagates TianoCore Bugzilla  <https://bugzilla.tianocore.org/>
>>>>> https://bugzilla.tianocore.org/ actions via email. Keep a close eye on
>> new
>>>>> issues reported for their assigned packages. Participate in triaging and
>>>>> analyzing bugs filed for their assigned packages.
>>>>> 3.	Responsible for reviewing patches and answering questions from
>>>>> contributors, on the edk2-devel mailing list
>>>>> <https://edk2.groups.io/g/devel/> https://edk2.groups.io/g/devel/.
>>>>> 4.	Responsible for coordinating patch review with co-maintainers and
>>>>> reviewers of the same package.
>>>>> 5.	Has push / merge access to the merge branch.
>>>>> 6.	Responsible for merging approved patches into the master branch.
>>>>> 7.	Follow the EDK II development
>>>>> <https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-
>>> Development-Pr
>>>>> ocess> process.
>>>>>
>>>>>
>>>>>
>>>>> IMHO, the 1~4 need technical expertise, while 5~7 need process
>> expertise.
>>>>>
>>>>> Logically, the can be two separated roles and be done by two different
>>>>> persons.
>>>>>
>>>>> A people who has strong technical expertise might NOT be the best
>> person
>>> to
>>>>> do the integration, and vice versa. I hope we can let right person do right
>>>>> thing in right way.
>>>>>
>>>>> For example, to avoid mistake during check in, 5~7 can be done by a role
>>>>> named “integrator”.
>>>>>
>>>>>
>>>>>
>>>>> My dream is that check-in process is just one click button. But it seems
>> we
>>>>> are still far from it…
>>>>>
>>>>>
>>>>>
>>>>> My two cents.
>>>>>
>>>>>
>>>>>
>>>>> Thank you
>>>>>
>>>>> Yao Jiewen
>>>>>
>>>>>
>>>>>
>>>>> From: devel@edk2.groups.io <mailto:devel@edk2.groups.io>
>>>>> <devel@edk2.groups.io <mailto:devel@edk2.groups.io> > On Behalf Of
>> Yao,
>>>>> Jiewen
>>>>> Sent: Saturday, September 26, 2020 1:09 PM
>>>>> To: devel@edk2.groups.io <mailto:devel@edk2.groups.io> ; Guptha,
>>> Soumya K
>>>>> <soumya.k.guptha@intel.com <mailto:soumya.k.guptha@intel.com> >;
>>>>> announce@edk2.groups.io <mailto:announce@edk2.groups.io>
>>>>> Subject: Re: [edk2-devel] Tianocore community page on who we are -
>> please
>>>>> review
>>>>>
>>>>>
>>>>>
>>>>> Thanks Soumya. I think this is a good start.
>>>>>
>>>>>
>>>>>
>>>>> Recently we are discussing the maintainer’s work in EDKII mailing list,
>>>>> with title “more development process failure”.
>>>>>
>>>>>
>>>>>
>>>>> I feel the process mentioned in
>>>>> https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-
>>> Development-Pro
>>>>> cess is not clear enough to follow, especially for the maintainer who is
>> not
>>>>> full time working on EDKII.
>>>>>
>>>>>
>>>>>
>>>>> I wish we can have this opportunity to revisit the “Follow the EDK II
>>>>> development
>>>>> <https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-
>>> Development-Pr
>>>>> ocess> process” and make “the process” simpler and clearer.
>>>>>
>>>>>
>>>>>
>>>>> Then all maintainers can sign to follow one rule. The rule we define and
>> the
>>>>> rule we agree with.
>>>>>
>>>>>
>>>>>
>>>>> Thank you
>>>>>
>>>>> Yao Jiewen
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> From: devel@edk2.groups.io <mailto:devel@edk2.groups.io>
>>>>> <devel@edk2.groups.io <mailto:devel@edk2.groups.io> > On Behalf Of
>>> Soumya
>>>>> Guptha
>>>>> Sent: Saturday, September 26, 2020 6:35 AM
>>>>> To: announce@edk2.groups.io <mailto:announce@edk2.groups.io> ;
>>>>> devel@edk2.groups.io <mailto:devel@edk2.groups.io>
>>>>> Subject: [edk2-devel] Tianocore community page on who we are -
>> please
>>> review
>>>>>
>>>>>
>>>>>
>>>>> Dear Community members,
>>>>>
>>>>>
>>>>>
>>>>> I have drafted a document “who we are”, explaining Tianocore
>> community
>>>>> structure, members of the community, their role and the current
>>> development
>>>>> process. I have drafted this document with the help of the Tianocore
>>>>> Stewards.
>>>>>
>>>>> We view this as a living document, as our development processes evolve,
>> I
>>>>> will keep this document updated.
>>>>>
>>>>>
>>>>>
>>>>> Please review the draft version of the document (link below) and provide
>>>>> your feedback. Please send it to me, no need to reply all.
>>>>>
>>>>> I appreciate your input by Friday, Oct 2. After this, I plan on make it live
>>>>> on our TianoCore wiki site.
>>>>>
>>>>>
>>>>>
>>>>> Link:
>> https://github.com/tianocore/tianocore.github.io/wiki/Who-we-are
>>>>>
>>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Soumya
>>>>>
>>>>>
>>>>>
>>>>> Soumya Guptha
>>>>> TianoCore Community Manager
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>
>>
>> 
>>
> 
> 
>