-
Notifications
You must be signed in to change notification settings - Fork 60
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
Fix for decoding reaction_added #83
base: master
Are you sure you want to change the base?
Conversation
Found another issue (see c4e2e9d) - so now this contains also a fix, not just a test. |
fabda13
to
61b2abb
Compare
Just noting since I forgot to previously: I am following this and I'll take another look once the upstream changes are sorted and we get travis passing. Thanks for the contributions! |
@@ -213,7 +213,7 @@ pub enum Event { | |||
user: String, | |||
reaction: String, | |||
item: Box<reactions::ListResponseItem>, |
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.
As mentioned in the PR on slack-rs-api
, this object should be replaced with the lighter-weight object as specified in the Slack event docs. No upstream change necessary! :)
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.
Makes sense, I'll do the changes later this week.
Related PR with fix: slack-rs/slack-rs-api#43.
Added test case based on a real message that could not be parsed.