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

Merged
merged 3 commits into from
Aug 4, 2024

Conversation

EdJoPaTo
Copy link
Contributor

@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
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
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
Contributor 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
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

Copy link
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.

@EdJoPaTo
Copy link
Contributor Author

As there seems to be not that much interest in it (silent for weeks) I'll just close this.

@EdJoPaTo EdJoPaTo closed this May 26, 2024
@EdJoPaTo EdJoPaTo deleted the edjopato/codeowners-team branch May 26, 2024 17:48
@EdJoPaTo EdJoPaTo restored the edjopato/codeowners-team branch August 2, 2024 08:26
@EdJoPaTo EdJoPaTo reopened this Aug 2, 2024
@EdJoPaTo EdJoPaTo requested a review from orhun August 2, 2024 08:36
@EdJoPaTo
Copy link
Contributor Author

EdJoPaTo commented Aug 2, 2024

I think this will also improve direct review requests. You can filter for review requests on your team or you as a person directly which might be neat.

This comment was marked as off-topic.

@orhun orhun merged commit 41a9100 into main Aug 4, 2024
34 checks passed
@orhun orhun deleted the edjopato/codeowners-team branch August 4, 2024 08:28
joshka pushed a commit to erak/ratatui that referenced this pull request Oct 14, 2024
…i#1081)

Use GitHub organization team in CODEOWNERS and create MAINTAINERS.md
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.

4 participants