Skip to content
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

REQUEST: Repository maintenance on opentelemetry-rust #2021

Closed
cijothomas opened this issue Mar 27, 2024 · 15 comments
Closed

REQUEST: Repository maintenance on opentelemetry-rust #2021

cijothomas opened this issue Mar 27, 2024 · 15 comments
Assignees
Labels
area/repo-maintenance Maintenance of repos in the open-telemetry org

Comments

@cijothomas
Copy link
Member

Affected Repository

https://github.com/open-telemetry/opentelemetry-rust
https://github.com/open-telemetry/opentelemetry-rust-contrib

Requested changes

Grant OpenTelemetryMaintainer access for @open-telemetry/rust-maintainers

Purpose

The maintainers currently do not have access to this repo to make certain CI checks required, thereby accidently merging PRs with failed CI. We'd like to get permissions to fix this.

Expected Duration

permanently (assume this is accepted practice!)

Repository Maintainers

@open-telemetry/rust-maintainers

@cijothomas cijothomas added the area/repo-maintenance Maintenance of repos in the open-telemetry org label Mar 27, 2024
@trask
Copy link
Member

trask commented Mar 27, 2024

assume this is accepted practice!

check out https://github.com/open-telemetry/community/blob/main/docs/how-to-configure-new-repository.md#collaborators-and-teams

If requested, foo-maintainers will be granted Admin permissions, and in return they must document any changes they make to the repository settings in a file named .github/repository-settings.md in their repository (other than temporarily disabling "Do not allow bypassing the above settings").

@cijothomas
Copy link
Member Author

@trask Thanks! We don't need admin privileges. We need the Maintain permissions only and permanently. I think it was just a bug that maintainers don't have it already!

The team foo-maintainers has Maintain permissions for the repository

@trask
Copy link
Member

trask commented Mar 27, 2024

Oops, yes, thanks

Cc @open-telemetry/technical-committee

@jack-berg
Copy link
Member

The @open-telemetry/rust-maintainers appear to already have the Maintain role for both opentelemetry-rust and opentelemetry-rust-contrib:

Screenshot 2024-03-27 at 3 12 26 PM Screenshot 2024-03-27 at 3 12 16 PM

The maintainers currently do not have access to this repo to make certain CI checks required, thereby accidently merging PRs with failed CI. We'd like to get permissions to fix this.

I don't think that changing the branch protection rules is covered by maintain. See github docs here:

People with admin permissions or a custom role with the "edit repository rules" permission to a repository can manage branch protection rules.

Typically people have either asked for TC to modify those, requested temporary admin to modify themselves, or requested permanent admin access and tracked the diff in .github/repository-settings.md.

@cijothomas
Copy link
Member Author

Thanks Jack! Let me get back with if we just ask TC to fix the CI checks, or need admin access temporarily and do it outselves.

Separately, something feels off here! Julian has "Maintainer" tag, though all other maintainers are part of the team... Any idea why so!?
image

Additionally, an emeritus-approver still in this list:
https://github.com/orgs/open-telemetry/teams/rust-approvers
image

But I, as a maintainer, does not seem to have the permission to remove emeritus approvers! And I don't see myself having permission to add someone to approver/maintainer either!

Tagging another maintainer, @TommyCpp, to see if this is just my account.

@trask
Copy link
Member

trask commented Mar 27, 2024

Julian has "Maintainer" tag

this means they are a maintainer of the team, and can add and remove members

it looks like that team wasn't set up quite right, here's how it should be set up: https://github.com/open-telemetry/community/blob/main/docs/how-to-configure-new-repository.md#collaborators-and-teams

someone from @open-telemetry/technical-committee can fix that for us

@hdost
Copy link
Contributor

hdost commented Mar 28, 2024

One thing we noticed when discussing this the other tday is that there's a "Maintain" and a custom role "OpenTelemetry Maintain" or something similar to that. I'm not sure what permissions that would grant.

@TommyCpp
Copy link

Tagging another maintainer, @TommyCpp, to see if this is just my account.

Can confirm I don't have access to manage the account in teams

@jack-berg
Copy link
Member

Separately, something feels off here! Julian has "Maintainer" tag, though all other maintainers are part of the team... Any idea why so!?

Fixed, I think something was just botched during initial team setup.

One thing we noticed when discussing this the other tday is that there's a "Maintain" and a custom role "OpenTelemetry Maintain" or something similar to that. I'm not sure what permissions that would grant.

I'm not sure about the origin of the OpenTelemetryMaintainer role, but it appears to be an extension of the base Maintainer role. Here is a screenshot of its permissions:

Screenshot 2024-03-29 at 10 29 01 AM

I compared to the Maintainer role and found the following additions:

  • Issue
    • Delete an issue
  • Repository
    • Manage deploy keys
    • Manage webhooks
  • Security
    • Delete code scanning analyses
    • View secret scanning alerts

I scanned ~10 or so repositories and didn't see OpenTelemetryMaintainer used in any of them.

Let me know what you all end up deciding with how you want to proceed with adding these CI checks.

@cijothomas
Copy link
Member Author

Thanks Jack. I can confirm it looks correct now for rust-maintainers team.

https://github.com/orgs/open-telemetry/teams/rust-approvers Could you fix this one too, as we still dont have ability to add/remove people here.

(Will get back on the CI check soon)

@jack-berg
Copy link
Member

https://github.com/orgs/open-telemetry/teams/rust-approvers Could you fix this one too, as we still dont have ability to add/remove people here.

All set. The rust-maintainers now all have the Maintainer role in rust-approvers.

@jack-berg
Copy link
Member

@cijothomas following up on this - are any other changes needed?

@cijothomas
Copy link
Member Author

@cijothomas following up on this - are any other changes needed?

Yes. Can you grant admin access to maintainers. We'll follow the below:

If requested, foo-maintainers will be granted Admin permissions, and in return they must document any changes they make to the repository settings in a file named .github/repository-settings.md in their repository (other than temporarily disabling "Do not allow bypassing the above settings", see branch protection rules below).

@jack-berg
Copy link
Member

Granted! Can you post a link to the .github/repository-settings.md when available as a paper trail?

@cijothomas
Copy link
Member Author

repository-settings.md

open-telemetry/opentelemetry-rust#1693

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/repo-maintenance Maintenance of repos in the open-telemetry org
Projects
None yet
Development

No branches or pull requests

5 participants