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

Adding the team to the CODEOWNERS #3898

Merged
merged 1 commit into from
Apr 6, 2018
Merged

Adding the team to the CODEOWNERS #3898

merged 1 commit into from
Apr 6, 2018

Conversation

karmel
Copy link
Contributor

@karmel karmel commented Apr 6, 2018

I'm not exactly sure what this will do, as Github's docs are unclear. In the ideal, this will round-robin someone, but it might actually just auto-assign everyone. If that's true, I will remove the team, and act as the primary triager for the time being. Though, of course, team members should tag relevant people to review if they know them (ie, secondaries or interested parties).

In any case, probably not necessary to have @k-w-w and @nealwu as reviewers on every PR anymore.

@karmel karmel requested review from nealwu, qlzh727 and k-w-w April 6, 2018 17:23
@@ -1,4 +1,5 @@
/official/ @nealwu @k-w-w @karmel
* @tensorflow/tf-garden-team
Copy link
Contributor

Choose a reason for hiding this comment

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

I think what you're going for here is that any PR modifying a very top-level file requests review from the garden team? If so I believe you want /* rather than just * which seems like it will just match everything.

Also keep in mind that CODEOWNERS itself is frequently modified when new research models are added (e.g., see the history), so this may happen a bit more often than you expected.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it should match any file except those that have rules in CODEOWNERS, and we should generally know about that-- especially if we are fielding issues for the new owners in question. Hooooopefully, this won't result in everyone getting tagged every time, but will magically choose one person. I kind of doubt that, but it's worth a try. If this doesn't work as intended, I will revert and just act as triager for the time being.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh that's right, the last match has priority so this will catch anything not covered. I think this may end up matching a decent number of PRs, but that's fine as we should fix that by adding in the appropriate owners.

@karmel
Copy link
Contributor Author

karmel commented Apr 6, 2018

Going to give it a try. Apologies to all of you in advance if this results in spamming.

@karmel karmel merged commit ea791e0 into master Apr 6, 2018
omegafragger pushed a commit to omegafragger/models that referenced this pull request May 15, 2018
@tfboyd tfboyd deleted the fix/codeowners branch February 7, 2019 18:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants