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@...>
抄送: gaoliming <gaoliming@...>; devel@edk2.groups.io;
Guptha, Soumya K <soumya.k.guptha@...>;
announce@edk2.groups.io; lersek@...; Kinney, Michael D
<michael.d.kinney@...>; 'Andrew Fish' <afish@...>
主题: 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@...>
Sent: Monday, September 28, 2020 8:02 PM
To: Yao, Jiewen <jiewen.yao@...>
Cc: gaoliming <gaoliming@...>; devel@edk2.groups.io;

Guptha,

Soumya K <soumya.k.guptha@...>; announce@edk2.groups.io;
lersek@...; Kinney, Michael D <michael.d.kinney@...>;

'Andrew

Fish' <afish@...>
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@...>;

Guptha,

Soumya K <soumya.k.guptha@...>; announce@edk2.groups.io
Cc: lersek@...; 'Leif Lindholm (Nuvia address)'

<leif@...>;

Kinney, Michael D <michael.d.kinney@...>; 'Andrew Fish'
<afish@...>
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@...>;

Guptha,

Soumya K <soumya.k.guptha@...>; 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@... <mailto:soumya.k.guptha@...> >;
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

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.

Thanks
Liming