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
Introduce chat.update bindings #112
Conversation
{ postMsgRspTs :: String | ||
{ postMsgRspTs :: Text |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a breaking change, but it is silly that this was a String to begin with.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will you be bumping the version number?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am going to write a changelog Soon and release 1.5 (current version is 0.5), but not in this PR.
@@ -59,7 +39,9 @@ $(deriveJSON (jsonOpts "postMsg") ''PostMsg) | |||
|
|||
data PostMsgReq = PostMsgReq | |||
{ postMsgReqChannel :: Text | |||
, postMsgReqText :: Text | |||
, postMsgReqText :: Maybe Text |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is actually Maybe in the API since you can have a message with blocks and no text.
authR <- mkSlackAuthenticateReq | ||
run (chatUpdate_ authR updateReq) . slackConfigManager | ||
|
||
chatUpdate_ :: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like I've asked this before about this file, but where are all these "underscored" functions defined?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lines 296 to 304 in ffeccec
apiTest_ | |
:<|> authTest_ | |
:<|> conversationsList_ | |
:<|> conversationsHistory_ | |
:<|> conversationsReplies_ | |
:<|> chatPostMessage_ | |
:<|> usersList_ | |
:<|> userLookupByEmail_ = | |
client (Proxy :: Proxy Api) |
-- | <https://api.slack.com/methods/chat.update> | ||
data UpdateReq = UpdateReq | ||
{ updateReqChannel :: ConversationId | ||
, updateReqTs :: Text |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, that's strange. do all messages in a channel have a unique timestamp?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, lmao. They are the primary key of messages, which is an Interesting design decision on the part of Slack, but I guess, there are many of those.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The breaking API change is not that bad, people will be able to fix their code easily. I just wonder if it's still a good idea to bump the version number?
No description provided.