-
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
#999 Adds Slack Connect API event types #1008
#999 Adds Slack Connect API event types #1008
Conversation
…g/bolt-js into sj-add-slack-connect-event-types
…g/bolt-js into sj-add-slack-connect-event-types
Codecov Report
@@ Coverage Diff @@
## main #1008 +/- ##
=======================================
Coverage 68.89% 68.89%
=======================================
Files 13 13
Lines 1212 1212
Branches 357 357
=======================================
Hits 835 835
Misses 304 304
Partials 73 73 Continue to review full report at Codecov.
|
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.
If reviewers think that it'd be best to merge these changes only after public docs are corrected, we can also hold off on merging.
This seems prudent to me - being able to reference something public in the PR that you based the work off of draws a nice line from the source to your changes. That said, I'm new on the team, so I am not familiar with how things are done 'round these parts yet, and I defer to what the rest of the team usually does 😄
@stevengill @filmaj's line of thinking I am following and liking, but defer to you on this in terms of waiting to merge these vs. prodding internally to get these corrected before finalizing these changes! |
…g/bolt-js into sj-add-slack-connect-event-types
@stevengill - Types have been updated in api.slack.com, and I've updated this PR with the changes. Should be finally ready for review! |
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.
LGTM!
Summary
Fixes #999 by adding event types for the Slack Connect APIs.
For reviewers: I wanted to take note of the fact that as of writing, the event type signatures listed on the api documentation site for the above are incorrect and need to be fixed (it's been noted internally).So I based these types on 1) internal examples of event payloads 2) technical spec and 3) confirmation from BE-engineer maintaining these endpoints. If reviewers think that it'd be best to merge these changes only after public docs are corrected, we can also hold off on merging.Reference types:
Requirements (place an
x
in each[ ]
)