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

Lessens dependence on English language #174

Open
wants to merge 5 commits into
base: master
from

Conversation

@TheLonelyGhost
Copy link
Collaborator

TheLonelyGhost commented Dec 2, 2019

Logic was discussed in modchat, but comparison of the Comment's subreddit will determine if it was a username mention or a directive to the bot (if not in the ToR subreddit, must be a mention).

Also includes some other cleanup stuff, such as relying on redis instead of python for if member of the blacklist set.

@TheLonelyGhost TheLonelyGhost requested a review from GrafeasGroup/core as a code owner Dec 2, 2019
tor/core/inbox.py Outdated Show resolved Hide resolved
send_to_modchat(
f'We received a message without an author -- '
f'*{item.subject}*:\n{item.body}', cfg
)
item.mark_read()

elif item.author.name == 'transcribot':
elif author_name == 'transcribot':
item.mark_read()

This comment has been minimized.

Copy link
@itsthejoker

itsthejoker Dec 2, 2019

Member

We can return here and just skip the rest of the processing since we know that we aren't going to do anything else to this comment.

This comment has been minimized.

Copy link
@TheLonelyGhost

TheLonelyGhost Dec 2, 2019

Author Collaborator

There's an argument that we could just add the transcribot user to the blacklist and not even need this at all too.

Realistically, the if-elif-else tree makes it so no other condition is evaluated already. Returning here (though I think you mean continue since it's a for-loop) would be redundant.

This comment has been minimized.

Copy link
@arfie

arfie Dec 3, 2019

Member

You could move item.mark_read() to the end of the function, since every branch calls it anyway. Then replace this one with just pass.

I also like the idea of blacklisting transcribot but that would probably need to be documented somewhere.

@arfie
arfie approved these changes Dec 3, 2019
item.mark_read()
continue
elif isinstance(item, RedditComment):
if str(item.subreddit.name).casefold() == str(cfg.tor.name).casefold():

This comment has been minimized.

Copy link
@arfie

arfie Dec 3, 2019

Member

Might also want to check here if the original post was made by the bot, since a user could mention the bot on a subreddit meta post and it would be interpreted as a regular command. The user interaction methods do already have guards for this in place so it wouldn't break anything, but by moving it here we could also reduce some duplicate code, and the logic becomes clearer.

send_to_modchat(
f'We received a message without an author -- '
f'*{item.subject}*:\n{item.body}', cfg
)
item.mark_read()

elif item.author.name == 'transcribot':
elif author_name == 'transcribot':
item.mark_read()

This comment has been minimized.

Copy link
@arfie

arfie Dec 3, 2019

Member

You could move item.mark_read() to the end of the function, since every branch calls it anyway. Then replace this one with just pass.

I also like the idea of blacklisting transcribot but that would probably need to be documented somewhere.

@@ -76,7 +76,6 @@ def process_reply(reply, cfg):
try:
if any([regex.search(reply.body) for regex in MOD_SUPPORT_PHRASES]):
process_mod_intervention(reply, cfg)
reply.mark_read()
return

This comment has been minimized.

Copy link
@arfie

arfie Dec 3, 2019

Member

Similar here, instead of returning every time, it could be a lot cleaner using if-elif?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.