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

Mirror changes in channel topic from IRC to Slack. #36

Merged
merged 3 commits into from
Oct 5, 2017

Conversation

bzh-bzh
Copy link
Contributor

@bzh-bzh bzh-bzh commented Oct 4, 2017

Solves part of #7

@bzh-bzh bzh-bzh requested a review from jvperrin October 4, 2017 05:55
@kkuehlz
Copy link
Member

kkuehlz commented Oct 4, 2017

Looks good

Copy link
Member

@jvperrin jvperrin left a comment

Choose a reason for hiding this comment

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

Nice, looks good, thanks!

@@ -101,6 +103,16 @@ def check_slack_rtm(self):
channel = self.channels[message['channel']]
return user_bot.post_to_irc('#' + channel['name'], message['text'])

def topicUpdated(self, user, channel, newTopic):
Copy link
Member

Choose a reason for hiding this comment

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

I'd name this something like update_topic instead to stay in the same style with the rest of the methods. Same with newTopic -> new_topic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an implementation of an event handler in IRCClient that gets called whenever the topic changes or a channel is joined for the first time, so I don't think it's possible to change the name of the function. I can change the topic variable name though.

Copy link
Member

@jvperrin jvperrin Oct 5, 2017

Choose a reason for hiding this comment

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

Oh, that's awkward, I forgot that's how it works. Maybe put a comment saying that it gets called by twisted when the topic is updated so it's clearer?

Thanks for updating the variable name though.

@bzh-bzh bzh-bzh merged commit 8df02c2 into ocf:master Oct 5, 2017
@bzh-bzh bzh-bzh deleted the motd_mirror branch October 5, 2017 07:53
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.

3 participants