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

Remove message object for chat.send #96

Open
nathanmsmith opened this issue Jan 5, 2019 · 4 comments
Open

Remove message object for chat.send #96

nathanmsmith opened this issue Jan 5, 2019 · 4 comments

Comments

@nathanmsmith
Copy link
Contributor

It feels a bit odd to me to have the ChatMessage type for use on the bot's chat.send method. For reference, ChatMessage looks like:

/**
 * A chat message. The content goes in the `body` property!
 */
export type ChatMessage = {|
  body: string,
|}

I think it's simpler for users to just input the text (body) of their message directly and let the chat.send method handle the formatting of arguments for them. Ideally, usage of chat.send would look like:

const channel = {name: 'kbot,' + bot.myInfo().username, public: false, topic_type: 'chat'}
bot.chat.send(channel, 'Hello kbot!').then(() => console.log('message sent!'))

@malgorithms @thebearjew thoughts? This would be a breaking change.

@malgorithms
Copy link
Contributor

I'm ok either way. I wouldn't worry about breaking changes since I think no one is using our module yet, and our README has warnings about its instability. But we can do an appropriate version upgrade.

I think the question to ask for this change is whether send will take anything other than body texts in the future. For example, will attachment sending be something like this?

bot.chat.send(channel, {
   file: "me.jpg",
   caption: "Hot selfie"
}).then...

these aren't "options" they're what is being sent.

Or will sending an attachment be an entirely different function call? In which case your change makes sense.

Hmm. how about this: we just wait a bit until wde do attachments and then decide

@nathanmsmith
Copy link
Contributor Author

Good point. I was thinking attachments would be a different method, since that's what we do in the command line

@nathanmsmith
Copy link
Contributor Author

Given that we now have a separate attach method, @malgorithms what do you think about this? Both the Go and Python bots just accept strings instead of the message object.

@malgorithms
Copy link
Contributor

sure, just make sure it's a big version upgrade since it'll break all existing bots.

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

No branches or pull requests

2 participants