-
Notifications
You must be signed in to change notification settings - Fork 6.6k
Update CODEOWNERS #2659
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 #2659
Conversation
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.
Definitely an improvement over what we had before. If we support multiple owners, that could also help for some of the feature areas.
.github/CODEOWNERS
Outdated
/pubsub/ @anguillanneuf | ||
/auth/ @busunkim96 | ||
/automl/ @nnegrey | ||
/bigquery/ @tswast |
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.
Maybe replace with Seth Hollyman as Tim is moving on to another team.
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.
Replaced mentions of tswast with shollyman
.github/CODEOWNERS
Outdated
/notebooks/ @alixhami | ||
/opencensus/ | ||
/profiler/ @jqll | ||
/pubsub/ @anguillanneuf |
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.
If we support multiple owners, maybe add hongalex 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.
Added
.github/CODEOWNERS
Outdated
/memorystore/ | ||
/ml_engine/ @alecglassford | ||
/monitoring/ | ||
/notebooks/ @alixhami |
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.
Torry also has contributed 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.
Added
.github/CODEOWNERS
Outdated
|
||
# Torry Yang is the primary maintainer of the Tables samples. | ||
/tables/ @sirtorry | ||
* @GoogleCloudPlatform/python-samples-owners |
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.
One issue we'll have to resolve at some point is how to handle reviewers from two different sets we discussed at the last meeting:
# Order is important; the last matching pattern takes the most # precedence. When someone opens a pull request that only # modifies JS files, only @js-owner and not the global # owner(s) will be requested for a review. *.js @js-owner
So this means that currently any changes to just the samples won't trigger/require a review from the "python samples readability" set.
One work around to this might be to d something like this for each entry:
/cloud-sql/ @kvg @GoogleCloudPlatform/python-samples-owners
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.
Done
.github/CODEOWNERS
Outdated
/bigquery/ @tswast | ||
/bigquery_storage/ @tswast | ||
/bigtable/ @billyjacobson | ||
/blog/ |
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.
Should we comment out folders that don't have specific owners? I'm not sure if leaving it blank explicitly changes it to "no-owner"
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.
Good catch, that seems to be the case, I've added python-samples-owners as an owner for every folder, so there are no longer blank folders.
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.
LGTM
@GoogleCloudPlatform/python-samples-owners Could you do a first pass through this list? After that I will open it up to individuals listed below.
This was roughly my thought process:
Inspect the recent history: