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

chore(github): use the GitHub organization team as codeowners #1081

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

EdJoPaTo
Copy link
Member

@EdJoPaTo EdJoPaTo commented May 2, 2024

This is mainly an idea on how to improve this as the Team can be managed by organization admins in the GitHub GUI.
Also, we get mentioned as team rather than individually which helps with organizing notifications.

Past maintainers could end up as a past maintainer team in the organization.

Thought about creating an issue about this first but why not discuss this directly on the change.

Past maintainers could end up as a past maintainer team in the organization

This comment was marked as off-topic.

@orhun
Copy link
Sponsor Member

orhun commented May 2, 2024

One thing that I don't like much is that this makes things less explicit. e.g. commits like 078e97e won't appear in the changelog.

we get mentioned as team rather than individually which helps with organizing notifications

Doesn't it work as same as before? Can you explain how this helps organizing the notifications?

Past maintainers could end up as a past maintainer team in the organization.

Does that mean we need to create another team for past maintainers? Or how does it work?

@kdheepak
Copy link
Collaborator

kdheepak commented May 2, 2024

One thing that I don't like much is that this makes things less explicit. e.g. commits like 078e97e won't appear in the changelog.

Do we need a separate file to track past and present maintainers? That information is useful for a potential user evaluating this package. In some projects I've worked on we've had a MAINTAINERS.md or CONTRIBUTORS.md file for that.

@orhun
Copy link
Sponsor Member

orhun commented May 2, 2024

Maybe it feels like a boomer thing to do but I think it's nice to have a file (or a place) to track the organizational changes both from security and transparency standpoint.

MAINTAINERS.md (or something similar) is a good idea, but currently we already have that functionality with CODEOWNERS. I'm okay with having MAINTAINERS.md and using the team name in CODEOWNERS though.

@EdJoPaTo
Copy link
Member Author

EdJoPaTo commented May 2, 2024

Doesn't it work as same as before? Can you explain how this helps organizing the notifications?

It might be the same because both would be a review request… I thought it's a person vs team mention but it isn't. 😒

@orhun
Copy link
Sponsor Member

orhun commented May 3, 2024

Yeah I see. Then I don't really see a huge benefit from this change - what the others think? @ratatui-org/maintainers

@joshka
Copy link
Member

joshka commented May 5, 2024

I think I'm with @EdJoPaTo on this one. Use the github team to manage users. I basically never check my github email notifications (as they're just a searchable backup of the site data for me), but I can empathize with those that do.

I suspect the x-github email headers for review requests will change to the team, which would make filtering easier, but I don't actually know for sure. Also, the list of reviewers in the top right will change to just the team.

X-GitHub-Sender: EdJoPaTo
X-GitHub-Recipient: joshka
X-GitHub-Reason: review_requested

I don't necessary think we need to worry about keeping maintainer history. Slap it in a markdown file somewhere (readme / contributors / credits / maintainers / thanks / something).

I'd like to see what happens with this, so slapping approve on it. I don't think there was a hard objection to this, and it's reversible, so if you feel like it makes sense to try go for it @EdJoPaTo

@joshka
Copy link
Member

joshka commented May 5, 2024

Copy link
Sponsor Member

@orhun orhun left a comment

Choose a reason for hiding this comment

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

I would prefer to have MAINTAINERS.md if we decide to go ahead with this.

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.

None yet

4 participants