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

Update owners #55

Merged
merged 4 commits into from
Dec 3, 2024
Merged

Update owners #55

merged 4 commits into from
Dec 3, 2024

Conversation

mrbojangles3
Copy link
Contributor

No description provided.

Copy link

github-actions bot commented Dec 2, 2024

🚀 Deployed on https://preview-55--hedgehog-docs.netlify.app

@github-actions github-actions bot temporarily deployed to pull request December 2, 2024 21:24 Inactive
Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

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

Overall looks good to me, in the sense that I'm happy to be codeowner for the repo.

As for the form, see also some comments in #53. Won't this change pull the three of us for each PR?

@github-actions github-actions bot temporarily deployed to pull request December 3, 2024 15:22 Inactive
Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

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

Single team for all changes to the repo, then?

@mrbojangles3
Copy link
Contributor Author

I am following your lead here as far as I understand it. Is the single team what you had in mind?

@qmonnet
Copy link
Member

qmonnet commented Dec 3, 2024

I am following your lead here as far as I understand it.

🥰

Is the single team what you had in mind?

So your initial version changed to:

* @Frostman @mrbojangles3 @qmonnet
docs @Frostman @sergeymatov @sonoble @thedvorkin @mrbojangles3

(I'm amazed I can pick syntax highlighting for CODEOWNERS files in GitHub comments)

One solution is indeed to use a single team for the single repo. This means that when someone opens a PR, and assuming we set up auto-assignments for reviews, we'll have one person from this team pulled for the review, no matter whether the changes are under docs or only to the build framework.

Something closer to your initial proposal would involve two teams:

* @githedgehog/docs-framework
docs @githedgehog/docs-contents

(Or I think we can even have subteams and something like @githedgehog/docs/framework if we prefer.)

Where @githedgehog/docs-framework would regroup Sergei, you and me, and @githedgehog/docs-contents would gather whoever has an interest in doing doc reviews.

This way, there would be one or two people pulled for each PR, enough to cover both owner teams (but possibly a single person if they happen to belong to the two teams, I'm not sure how GitHub works in details on that side). This would avoid pulling Mike or Steven when we change the CI job, for example.

Side note: One drawback of having a single person from a team pulled for review is that the PR is blocked as long as that person doesn't do the review (or unless the author pings someone else from the team). So this means whoever joins the teams must be ready to do the reviews within a reasonable delay.

Personally, I'd be in favour of having these two teams, but others may disagree. Let's discuss at the call today?

Copy link
Member

@Frostman Frostman left a comment

Choose a reason for hiding this comment

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

+1 for @qmonnet proposal on 2 teams

@qmonnet qmonnet linked an issue Dec 3, 2024 that may be closed by this pull request
@github-actions github-actions bot temporarily deployed to pull request December 3, 2024 20:57 Inactive
@qmonnet
Copy link
Member

qmonnet commented Dec 3, 2024

We need both teams to have write access to the current repo for the new CODEOWNERS file to be valid. @Frostman can you please help with that?

@qmonnet qmonnet requested a review from Frostman December 3, 2024 21:10
@Frostman Frostman merged commit 0035ac5 into master Dec 3, 2024
2 checks passed
@Frostman Frostman deleted the update-owners branch December 3, 2024 21:12
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.

Use dedicated reviewer teams in CODEOWNERS
3 participants