-
Notifications
You must be signed in to change notification settings - Fork 399
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 #926 by adding more subtype ones to message event types #928
Conversation
Codecov Report
@@ Coverage Diff @@
## main #928 +/- ##
=======================================
Coverage 66.22% 66.22%
=======================================
Files 13 13
Lines 1205 1205
Branches 355 355
=======================================
Hits 798 798
Misses 338 338
Partials 69 69 Continue to review full report at Codecov.
|
src/types/events/message-events.ts
Outdated
@@ -67,6 +76,104 @@ export interface BotMessageEvent { | |||
thread_ts?: string; | |||
} | |||
|
|||
interface ChannelArchiveMessageEvent { |
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 other interfaces in this file are exported. Should we follow suit with the new ones, as well?
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.
Thanks! Fixed.
event_ts: '', | ||
channel_type: 'channel', | ||
}; | ||
assert.isNotEmpty(payload); |
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.
Interesting. I've never seen types tested this way!
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.
We can do the same in the types-tests
but having these is easier. Also, we can add other kinds of tests in the same place too.
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 defer to you on this if you think it best. Testing types is not something I'm well-versed in enough to have a strong opinion (yet!). 🙂
d895ce5
to
ea6a725
Compare
event_ts: '', | ||
channel_type: 'channel', | ||
}; | ||
assert.isNotEmpty(payload); |
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.
We can do the same in the types-tests
but having these is easier. Also, we can add other kinds of tests in the same place too.
@@ -67,6 +76,104 @@ export interface BotMessageEvent { | |||
thread_ts?: string; | |||
} | |||
|
|||
export interface ChannelArchiveMessageEvent { |
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.
@misscoded Thanks for pointing the lack of export out here! Fixed.
Summary
This pull request resolves #926
Requirements (place an
x
in each[ ]
)