-
Notifications
You must be signed in to change notification settings - Fork 664
[ci] Clean-up and simplify CODEOWNERS #8066
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
base: main
Are you sure you want to change the base?
Conversation
The existing setup-up of groups was hard to maintain because of the many nested groups. This makes it much more clear. Additionally, there were a few directories that only had a single maintainer. This ensures the core team is on every directory, and then adds people ass necessary.
|
||
# Global rule: | ||
* @wpilibsuite/wpilib | ||
* @PeterJohnson @ThadHouse @calcmogul @rzblue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't this cause GitHub to tag all the individuals and give the impression all named individuals need to approve the PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It sounds like it will only require one approval per match pattern
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but my experience is that's super non-obvious
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you worried about it from the reviewers perspective, or worried that a PR author will go an follow up with a bunch of people?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The latter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if you configure new teams in the GitHub organization and use them here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes we discussed this on Thursday and that’s the plan.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The flipside is that using the groups makes it harder to see who the real reviewers are, as seen in this PR: #8219
The existing setup-up of groups was hard to maintain because of the many nested groups, and not very visible. This makes it much more clear.
Additionally, there were a few directories that only had a single maintainer. This ensures the core team is on every directory, and then adds people as necessary.