Skip to content
This repository has been archived by the owner on Mar 11, 2024. It is now read-only.

Add ChannelData to Cmd #41

Merged
merged 7 commits into from
Feb 21, 2017

Conversation

doenietzomoeilijk
Copy link
Contributor

This stores a bit more info than just the channel name; in addition you get the protocol (irc, slack, telegram), server host, and whether the channel is a private chat or a public channel.

As of now this is a drop-in replacement without BC breaks.

Also relevant to #6 and #37 .

This stores a bit more info than just the channel name; in addition you get the protocol (irc, slack, telegram), server host, and whether the channel is a private chat or a public channel.

As of now this is a drop-in replacement without BC breaks.
slack/slack.go Outdated
@@ -67,7 +69,14 @@ Loop:
readBotInfo(api)
case *slack.MessageEvent:
if !ev.Hidden && !ownMessage(ev.User) {
b.MessageReceived(ev.Channel, ev.Text, extractUser(ev.User))
ti, _ := api.GetTeamInfo()
Copy link
Member

@fabioxgn fabioxgn Feb 17, 2017

Choose a reason for hiding this comment

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

This team info could be obtained after connecting to slack to avoid making a request for each received msg couldn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're absolutely right - will fix!

@doenietzomoeilijk
Copy link
Contributor Author

I see I have left out the proper info for Slack channels, please hold off on merging this PR.

@doenietzomoeilijk
Copy link
Contributor Author

Added some code to get the proper channel name for Slack channels, rather than just some Cxxxxxx code. I'm not 100% sure if this is the most effective way - am I not pissing away memory by storing / retrieving the Channels this way? Should or shouldn't I use pointers? Some guidance would be nice, since I'm still new to Go. :)

While adding this code, I saw that extractUser doesn't seem to do any sort of caching - is that really what we want to do, or should we cache the User ID's with the attached Slack User, similar to what I'm doing with Channels now? I could use the user_change event to update our list in case of user profile updates. Let me know if that's desired.

@fabioxgn
Copy link
Member

@doenietzomoeilijk You are right, the extractUser could be cached, if you are willing to change it will be a nice optimization.

And I don't think this list of channels would be an issue to be stored, go uses so little memory that you must try harded than this for it to be an issue 🤣

Thanks for your PR, I'll merge it and deploy this new changes to my bot to test it out.

@fabioxgn fabioxgn merged commit e614e12 into go-chat-bot:master Feb 21, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants