On Nov 18, 2019, at 7:04 PM, Christian Daudt Christian.Daudt@cypress.com wrote:
Kumar Gala wrote:
On Nov 18, 2019, at 5:36 PM, Christian Daudt via TSC tsc@lists.trustedfirmware.org wrote:
Step 2: All tsc representatives will be granted +2/merge permissions into trusted-firmware-m. Allowable uses of this +2 are as follows: a) For gerrit reviews that exclusively involve changes to files+directories where the tsc rep's company is listed as maintainer in the MAINTAINERS file b) For gerrit reviews that involve changes to files+directories where the tsc rep's company is listed as maintainer in the MAINTAINERS file + other changes: only once other maintainers have +1 their portions (i.e. once all maintainers have +1, any of the companies involved in the patch can +2/merge it). Abuse of this privilege will be escalated to board for resolution.
Step 3: on any patch that requires a change to maintainers (i.e. a new platform being added): a) the patch will include an update to the MAINTAINERS file indicating it. b) the maintainers will add themselves to get notifications from gerrit for appropriate subtrees.
Responsibility of maintainers:
- Ensure MAINTAINERS file and your gerrit notifications are kept up-to-date
- Provide review in a timely fashion for any reviews that involve your subtree (preferably within a week)
- Notify your tsc rep once a review has sufficient approvals to be +2ed.
Let me know of any comments, concerns, issues. If there is agreement on the above, I'll outline the steps with more detail + examples (in phabricator possibly or as a documentation patch to the code itself). Thanks, Christian.
STEP 2/3: I don’t think TSC reps should be given this permission by default. I think the TSC should vote/approve any Maintainer. The reason for this is to have things be a meritocracy.
The rules for approval w/regards to +1/+2 should be around MAINTAINERs regardless who they work for.
I think at this point of the project that hopefully the TSC would approve a MAINTAINER proposed by a Silicon Vendor for the Silicon Vendor’s specific code.
- k
The intention around this permission is not to give auto-maintainer status to anyone - it is an attempt to bolt a linux-style maintainership with the realities of the tool (gerrit). It is a somewhat separate item from maintainership to try to scale ARM devs who are the only ones with +2 access to the repo at present. The intent here is that, for maintainers that are employed by a member company, the tsc rep can push the code (after the maintainer approves it) to reduce the load on the ARM +2ers. Non-member maintainers are not precluded from being added in any way - my original intention is that responsibility of +2/merging on behalf of the non-member maintainers would remain with ARM folks (as it it today for all patches) - but we can revisit those rules once we have some non-member maintainers ready to be admitted. If there is a better way to synchronize the intention (of distributing some of the maintainership) with the tool at hand (gerrit) I'd be happy to incorporate that into the proposal.
Thanks, Christian.
PS: I manually did the reply indentation - hope it worked. Sigh outlook…
I guess the question is if we separate the review/approval role from the merging role. If that’s the intent then I think we should define such roles.
Maintainer: Reviews/approves code to merge Merger: Merges code that has been approved, is aware of state of project if it makes sense to review (ie, if in a release window, might not make sense to merge code even if its approved).
We are actually going through a similar discussion / process in the Zephyr project.
PS: Outlook munged it, but its workable :)
- k