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

Add maintainers file #167

Merged
merged 1 commit into from
Nov 28, 2023
Merged

Conversation

afrittoli
Copy link
Contributor

Changes

Add maintainers file with the existing members of the spec-maintainers team

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

Copy link
Contributor

@e-backmark-ericsson e-backmark-ericsson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how this file relates to the built-in support for a CODEOWNERS.md file. They might be mutually exclusive, and the CODEOWNERS file could refer to a GitHub team and therefore there is no need to explicitly specify the individual names in it.

@afrittoli
Copy link
Contributor Author

I'm not sure how this file relates to the built-in support for a CODEOWNERS.md file. They might be mutually exclusive, and the CODEOWNERS file could refer to a GitHub team and therefore there is no need to explicitly specify the individual names in it.

In my understanding CODEOWNERS is used for automatic selection of reviewers based on changes in repo files and folders. I was looking for a native file based way to assign roles to users but I didn't find one, or at least it didn't seem to me that code owners would do that.

The mechanism I proposed in the other PR involves both changing this file and adding the user to a team, the first part allows us to have a PR that tracks the changes, the latter actually grants roles and permissions.

@afrittoli
Copy link
Contributor Author

afrittoli commented Nov 25, 2023

I'm not sure how this file relates to the built-in support for a CODEOWNERS.md file. They might be mutually exclusive, and the CODEOWNERS file could refer to a GitHub team and therefore there is no need to explicitly specify the individual names in it.

In my understanding CODEOWNERS is used for automatic selection of reviewers based on changes in repo files and folders. I was looking for a native file based way to assign roles to users but I didn't find one, or at least it didn't seem to me that code owners would do that.

The mechanism I proposed in the other PR involves both changing this file and adding the user to a team, the first part allows us to have a PR that tracks the changes, the latter actually grants roles and permissions.

I'm happy to add a CODEOWNERS file nonetheless. I removed maintainers.md and added the extra info in the in comments inside CODEOWNERS.

@e-backmark-ericsson
Copy link
Contributor

In Eiffel we handle this solely through GitHub teams. Our CODEOWNERS files only point out the GitHub team and then maintainers are added/removed from the repo by updating the team only. Yes, we loose the tracking/review of PRs to update maintainers, but to me that is fine. Emeritus maintainers would then need to be handled in a separate file (or still as comment in CODEOWNERS if we like).

Example: https://github.com/eiffel-community/eiffel/blob/master/CODEOWNERS

@afrittoli
Copy link
Contributor Author

We could add the team to the CODEOWNERS file, but I like the idea of having an auditable record of the change being approved.
We could also use a GitHub issue instead of a PR, if you prefer to have teams in the CODEOWNERS files

@e-backmark-ericsson
Copy link
Contributor

I'm slightly concerned that we will need to update the same information in multiple places (CODEOWNERS file and GitHub team). I'm pro centralizing such operations as much as possible. But I see the point of having PRs (or issues) to track it as well.
I don't have a strong opinion on this so we could let today's meeting decide :)

The CODEOWNERS file can be parsed by GitHub to suggest reviewers for
PRs. The actual permsissions are defined in the GitHub configuration.

More details about maintainers and emeritus are tracked as comment.

Signed-off-by: Andrea Frittoli <[email protected]>
@afrittoli
Copy link
Contributor Author

I'm slightly concerned that we will need to update the same information in multiple places (CODEOWNERS file and GitHub team). I'm pro centralizing such operations as much as possible. But I see the point of having PRs (or issues) to track it as well.

Thank you @e-backmark-ericsson - as discussed during the WG, I switched the automation part to the team, and kept the table in the comment to make it easier for people to discover the list of maintainers.

@afrittoli afrittoli merged commit dd2703b into cdevents:main Nov 28, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants