-
Notifications
You must be signed in to change notification settings - Fork 178
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 CODEOWNERS #3331
base: main
Are you sure you want to change the base?
Update CODEOWNERS #3331
Conversation
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
libcudacxx/ @nvidia/cccl-libcudacxx-codeowners @nvidia/cccl-codeowners | ||
thrust/ @nvidia/cccl-thrust-codeowners | ||
cub/ @nvidia/cccl-cub-codeowners | ||
libcudacxx/ @nvidia/cccl-libcudacxx-codeowners | ||
cudax/ @nvidia/cccl-cudax-codeowners | ||
c/ @nvidia/cccl-c-codeowners |
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.
I think Python team should be a stakeholder here
c/ @nvidia/cccl-c-codeowners | |
c/ @nvidia/cccl-c-codeowners @nvidia/cccl-python-codeowners |
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.
(alternatively, we could just nuke cccl-c-codeowners
...)
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.
I agree there's a lot of overlap, but I've just encoded that in the membership of the two different groups. Putting two teams listed as codeowners would end up requiring 4 total reviews which is probably too many.
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.
I thought the threshold is set independently in the repo setting, not based on the number of teams? (I could be wrong.)
libcudacxx/ @nvidia/cccl-libcudacxx-codeowners @nvidia/cccl-codeowners | ||
thrust/ @nvidia/cccl-thrust-codeowners | ||
cub/ @nvidia/cccl-cub-codeowners | ||
libcudacxx/ @nvidia/cccl-libcudacxx-codeowners | ||
cudax/ @nvidia/cccl-cudax-codeowners |
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.
Note: by the time we start integrating some components from cudax with cuda.core, we might need Python team here. For now it's OK since I keep an eye on it already.
Update the CODEOWNERS in preparation for enabling 2 required reviews.
I've changed the strategy from how it was set up before where it would assign reviewers from a general pool as well as a specific pool. This was more relevant when the team was smaller. Now that we have more people, I think we can just require 2 reviewers from the specific codeowner team.
For reference, here is the current group membership: https://github.com/orgs/NVIDIA/teams/cccl-codeowners/teams
This should be updated as well.