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

Notify proxy maintainers #1167

Merged
merged 17 commits into from
Feb 27, 2020
Merged

Notify proxy maintainers #1167

merged 17 commits into from
Feb 27, 2020

Conversation

gabrieldemarmiesse
Copy link
Member

@gabrieldemarmiesse gabrieldemarmiesse commented Feb 27, 2020

Fix #1145

@gabrieldemarmiesse gabrieldemarmiesse requested a review from a team as a code owner February 27, 2020 17:00
@boring-cyborg boring-cyborg bot added the github label Feb 27, 2020
@bot-of-gabrieldemarmiesse

@tensorflow/sig-addons-maintainers

You are owners of some files modified in this pull request.
Would you kindly review the changes whenever you have the time to?
Thank you very much.

@gabrieldemarmiesse
Copy link
Member Author

Hooray!

@gabrieldemarmiesse
Copy link
Member Author

gabrieldemarmiesse commented Feb 27, 2020

I suppose we need to remove all people who have write access, right? They already get a notification. and also tensorflow/sig-addons-maintainers since it doesn't seem to trigger anything: #1137 (comment)

@seanpmorgan
Copy link
Member

I suppose we need to remove all people who have write access, right? They already get a notification. and also tensorflow/sig-addons-maintainers since it doesn't seem to trigger anything: #1137 (comment)

Thanks this is fantastic!

  1. Yeah we'll probably want to remove the duplicate notifications. Do you have a method in mind for doing this? Filtering members of sig-addons-maintainers?

Subsequent PR:

  1. Yeah sig-addons-maintainers doesn't actually do anything. We can remove that catch all.
  2. We'll need to update the CODEOWNERS to match the README docs. Then we can remove the maintainers from the README so we don't have duplicated work on PRs.

@gabrieldemarmiesse
Copy link
Member Author

gabrieldemarmiesse commented Feb 27, 2020

The easiest is to hard-code in the python file the people with write access. This is not public information so I cannot fetch it automatically. I'll add a filter somewhere.

We want to minimize spamming. So the bot will only notify when the pull request is opened. Never after. It's also simpler code-wise.

Copy link
Member

@seanpmorgan seanpmorgan left a comment

Choose a reason for hiding this comment

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

LGTM Thanks again!

@seanpmorgan seanpmorgan merged commit acf362c into tensorflow:master Feb 27, 2020
@gabrieldemarmiesse gabrieldemarmiesse deleted the notify_proxy_maintainers branch March 8, 2020 16:32
jrruijli pushed a commit to jrruijli/addons that referenced this pull request Dec 23, 2020
* Made the bot code.
* Used a blacklist.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Find a way to label PRs with the relevant code owners.
4 participants