-
Couldn't load subscription status.
- Fork 93
feat: Define Permissions of Global Maintainers #485
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Summary of ChangesHello @aepfli, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request formalizes the introduction of a 'Global Maintainer' role within the project's contributor ladder. The primary goal is to empower highly trusted and experienced contributors with elevated permissions, enabling them to provide more effective support to the community, particularly in facilitating rapid merges for critical fixes and overseeing cross-repository technical decisions. This change aims to streamline project operations and enhance overall community support by distributing some administrative capabilities beyond just the TSC and Governing Board. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a new 'Global Maintainer' role to the contributor ladder, defining their requirements, responsibilities, and the process for becoming one. The changes are well-structured, but I've found a couple of areas that need clarification to ensure the governance process is clear and consistent. Specifically, there's an incomplete sentence in the approval process and an inconsistency regarding who can object to a nomination. My detailed comments provide suggestions to address these points.
23c8499 to
7f6c0ab
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aepfli I am approving, but please wait for one additional approval from each of both the TC and the GC, excluding Dynatrace employees.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to me. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to me! Left one question.
Signed-off-by: Simon Schrottner <[email protected]> Signed-off-by: Todd Baert <[email protected]>
d3e9939 to
10b2ef2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am approving, and I think I agree with this in concept.
I do have questions though. Is this proactive, or an attempt to address issues that have been encountered?
Is there anything more nuanced we can do, like two global maintainers to bypass checks?
Do we have any limitations on distribution of global maintainers? Number of them, numbers employed by specific companies, etc?
There's been a few times where things like broken CI checks prevent us from releasing, or fixing otherwise trivial issues. None of them have been "911's" or security issues or anything like that, but they suggested to me having a few more trusted people with override access might not be a bad idea. We also have a few maintainers that regularly work on release automation, but don't have access to the credentials associated with publishing - it makes it really hard for them to work on such tasks independently.
Perhaps, but at a certain point it might defeat the purpose; for now, I would just say we leave this role to highly trusted individuals, and make clear what the expectations are for any significant bypasses.
No; that was one of the reasons I tried to make clear that there's no decision-making power associated with this role (unlike the GC/TC); this role is purely operational, so IMO I don't think there's as much concern on conflict of interest. @kinyoklion please let me know if you disagree with any of my reasoning; I'd definitely like consensus on this. |
So my main concern is the broad power of these roles, and maybe there isn't a good way to have a better balance. In the work I do day-to-day I have less power in the repositories that I work in than this role will have. Breaking glass still requires at least 2 people in most circumstances. Which isn't a matter of trust, but of robustness of the overall system. The difference between a compromised SSH credential being a major problem or simply a nuisance. |
I would be in favor of a requirement to force 2 people to bypass checks, but I don't think this is possible in Github (I think all the bypass operations are just big red buttons only 1 person need push (assuming they have admin rights). @aepfli Do you have any proposals? @kinyoklion is correct that we are certainly increasing the security surface area here in terms of who has admin rights. |
We may need to adjust our rulesets to allow bypassing (https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-rulesets/creating-rulesets-for-a-repository#granting-bypass-permissions-for-your-branch-or-tag-ruleset). I don't have the necessary permissions, so I'm unable to view the current configuration. I think GitHub doesn't have any functionality for 2 people approving a bypass (that would be the same as asking 2 people to approve a PR). Bypass is for "very urgent" factors only. |
|
I do not have insights into our role definitions, and/or what we can do organizational wise, but we have encountered issues, where eg. in java by error a binary incompatibility got released - hard to grasp at the beginning. currently we do need two approvers for a pull request to get merged - but there is only a limited amount of people who could bypass the automation to rollback this change. I agree that we increase the attack surface, but maybe we find a solution, which allows only global-maintainers to bypass reviewer limitations for mitigating such issues faster. To be honest I do not want to gain permissions to push to main, nor do i want to adapt those settings, but as a maintainer, I am currently not able to fix those issues. To be honest, by now i would have applied for the TSC but i think this would create an unfair balance (and might not even be allowed due to our definitions) as Dynatrace does have already a seat in the TSC. To be honest i am fine if we do not increase the permissions, but i also do see currently a big dependency on tasks to the TSC and GB although there expertise is not really needed. eg. having enough approvers, or moving forward with a task, even if it is a small one. |
| - Must have been a maintainer in at least one sub-project for 6 months | ||
| - Significant contributions across multiple repositories and programming languages | ||
| - Demonstrated ability to collaborate with and mentor contributors across the community | ||
| - Nominated by an existing Global Maintainer or a majority of sub-project maintainers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe also nominated by TSC?
Adding a definiton for global maintainers.
I do see the global maintainers as the support crew for TSC and GB - sometimes things need to be merged fast and bypass certain automations. Eg. small fixes, etc. Currently only the TSC and GB can do this, due to their ADMIN rights.
Global maintainers have proven to be reliable and trustworthy, hence i think we should grant those more rights, to support the community better.
Signed-off-by: Simon Schrottner [email protected]