Hi Folks,
Thanks for good discussions around this topic.
The purpose of this document "Who we are" is intended to remain high level to introduce the community members and their roles. Please note that some of the feedback is very detailed that probably fits into the TianoCore
development process document.
Below are my proposed changes to the document based on the emails.
Please review and let me know if you see any issues by Oct 5. Please also directly edit the document and let us know what you edited in the document.
Fyi.. my plan is for this page to go live on Oct 9th. This will be a living document and we can make changes as we discover more.
I have added a new member "Release Manager", added TianoCore admin role, added responsibilities to the Maintainer and Reviewer section.
I agree that Maintainer is the one who approves final patch. I see the argument for creating ¡§Approving Reviewer" and "Assistant Reviewer¡¨ roles, I am holding off this proposal to discuss in the upcoming Stewards meeting and make a call.
Release Manager
Role/Responsibilities:
1)The Release Manager is responsible for driving the quarterly Stable Tags. The Release Manager will plan the features, schedule the release date, create the Stable Tag with the release notes and announce to the EDK2 community on the
release milestones: Soft feature freeze, hard feature freeze and the final release of the Stable Tag.
Maintainer Responsibility
1)Maintainer or Reviewer is responsible for timely responses on emails addressed to them (preferably less than calendar week).
Reviewer Responsibility
1)Reviewer or Maintainer is responsible for timely responses on emails addressed to them(preferably less than calendar week).
2) Open ¡V Add Approving Reviewer" and "Assistant Reviewer".
TianoCore Admin:
Role: approve/remove access to TianoCore resources such as GitHub, Bugzilla, groupsIO etc..
Responsibilities:
Respond to emails and monitor role changes in the community (adding/removing maintainers..)
The request to add the below - Contributor responsibilities. This is too detailed. I would add this to the development process document.
CONTRIBUTOR
Responsibilities:
If a contributor proposes an incompatible change, the contributor should coordinate with the platform maintainer and make an agreement on who will follow up to update the impacted platforms before merging the patch. The impacted platforms
include everything in Edk2 and Edk2Platforms.
Thanks,
Soumya
-----Original Message-----
From: Leif Lindholm <leif@nuviainc.com>
Sent: Thursday, October 1, 2020 3:22 AM
To: Laszlo Ersek <lersek@redhat.com>
Cc: gaoliming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>; Guptha, Soumya K <soumya.k.guptha@intel.com>; announce@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>; 'Andrew Fish' <afish@apple.com>
Subject: Re: ¦^Î`: [edk2-devel] Tianocore community page on who we are - please review
On Thu, Oct 01, 2020 at 10:44:10 +0200, Laszlo Ersek wrote:
> On 09/30/20 12:13, Leif Lindholm wrote:
> > Agreed.
> >
> > Reviever or Maintainer can approve a patch. Any Maintainer can push
> > a patch that has been approved.
>
> I disagree.
>
> Assume Ard and myself are away and Jordan fails to report back in a
> week or so, but Rebecca or Peter have reviewed a patch on the list for
> OvmfPkg/Bhyve.
>
> In that case, the patch should *NOT* be merged by (for example) you,
> just because you have push rights. The community will have to wait
> until Ard, Jordan, or myself return and provide an ACK.
>
> If the maintainers are *consistently* irresponsive, then new
> maintainers need to be added, possibly with a larger community
> discussion. But if it's just a week (especially if we discussed our
> absence in advance), then maintainer absence is completely sufficient
> and justified for holding back patches, even if designated reviewers
> are OK with those patches.
>
> I've been *really* disliking that, for example, the chief MdeModulePkg
> reviewers don't regularly ACK patches that have been reviewed by
> designated reviewers. If those reviewers are considered authoritative
> enough to fully approve patches -- and most of them they have push
> access already, anyway --, then we should rework Maintainers.txt so
> that Maintainer roles be handed out with a finer granularity. If you will:
> promote those reviewers to Maintainers, on their respective turfs.
>
> > This can happen either:
> > - when the designated Maintainer for that patch is
> > unavailable/unresponsive
> > - if the patch submitter is also a Maintainer of some other part of
> > the repo.
> >
> > No one can approve their own patches.
> >
> > The act of adding a Reviewer means delegating the approval work to
> > them.
>
> I don't see it like that; I think Maintainers should have the last
> word on every patch going in. And yes, this *requires* maintainers to
> be responsive.
>
> ... Hm. Perhaps this is a sign that we really have two concepts here,
> we've just not been distinguishing them clearly enough. Maybe we need
> to split the reviewer role in two: "Approving Reviewer" and "Assistant
> Reviewer".
I think you're right. This is why we seem to have two sets of opinions on this topic.
> For example, on OvmfPkg, I would suggest marking all current Reviewers
> as "Assistant Reviewers". On ArmVirtPkg, I'd likely propose you as an
> Approving Reviewer (you have stood in for Ard and myself anyway, for
> years now), and suggest Assistant Reviewer role for Julien.
Right, that makes sense to me.
If I was to start bikeshedding, I might suggest adding an A: tag for approving reviewer. Possibly stealing the description from the current
R: tag, and adding the approving bit. And maybe nicking the QEMU R:
description outright for R:.
> On
> MdeModulePkg and other core packages, I'd defer the re-classification
> to Intel; we'd likely see a really large number of Approving Reviewers
> (justifiedly, I think).
Agreed.
/
Leif