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

Updated main.py to add an exclude array for users #1

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Updated main.py to add an exclude array for users #1

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Jun 6, 2019

Updated main.py to add an exclude array for users so that community members can explicitly opt out as requested by the community.

Updated main.py to add an exclude array for users so that community members can explicitly opt out.
Copy link

@zoidyzoidzoid zoidyzoidzoid left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! 🙏 🎉

I think once you've addressed my minor comments, we could merge this.

@@ -45,6 +48,8 @@ def process_message_event(client, event):
return
user = event['user']
if any(hit in text.lower() for hit in HITS):
if any(excludeUser.lower() in user.lower() for excludeUser in EXCLUDE_USERS):

Choose a reason for hiding this comment

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

We could exit out even earlier in the function, since we don't need to check if they said something that triggers the bot's response, since it'll be faster and avoid the other work. I'd also suggest keeping the excluded users in a Python set instead of array, then we can just check if user.lower() in EXCLUDE_USERS.

@@ -12,6 +12,9 @@
from requests import post
from slackclient import SlackClient

EXCLUDE_USERS = [
'UC2PXG134'

Choose a reason for hiding this comment

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

We should maybe put these in some sort of configuration file, since other folks are starting to use this bot on their own workspaces and potentially Slack user IDs could collide maybe? I also think including their username in a comment is useful, in case folks ask to get added/removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant