-
Notifications
You must be signed in to change notification settings - Fork 275
doc: make events in the document clearer #415
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
Conversation
Hi @RhnSharma, thanks for taking the time to make this pull request! Perhaps, there may be more to change in the documents. I would like other maintainers to check this first. |
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.
Hey @RhnSharma!
Thank you so much for sending this in! I left a few comments on other instances of events we can update.
Also, to close the issue, we should look into updating these files as well.
- https://slack.dev/bolt-python/concepts#action-listening - this references acknowledging events section and also uses the word events which should be changes to requests
- https://slack.dev/bolt-python/concepts#action-respond - uses event, should be replaced to request
- https://slack.dev/bolt-python/concepts#adapters - three instances of events -> requests
- https://slack.dev/bolt-python/concepts#authorization - three instances of events -> requests
- https://slack.dev/bolt-python/concepts#global-middleware - once instance of events -> requests
- https://slack.dev/bolt-python/concepts#context - two instances to update
- https://slack.dev/bolt-python/concepts#lazy-listeners - three of the four instances should be updated to requests. I'd leave
For events, while a listener doesn’t need ack() method
events reference alone
ack(response_action="errors", errors=errors) | ||
return | ||
# Acknowledge the view_submission event and close the modal | ||
# Acknowledge the view_submission request and close the modal |
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.
Line 21 in this file also says view_submission event
. Same with line 10. Could you update these as well?
Your app can use the `command()` method to listen to incoming slash command events. The method requires a `command_name` of type `str`. | ||
|
||
Commands must be acknowledged with `ack()` to inform Slack your app has received the event. | ||
Commands must be acknowledged with `ack()` to inform Slack your app has received the request. |
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.
Line 10 in this file also uses events. Could you update that
incoming slash command events
Shortcuts are invokable entry points to apps. Global shortcuts are available from within search and text composer area in Slack. Message shortcuts are available in the context menus of messages. Your app can use the `shortcut()` method to listen to incoming shortcut events. The method requires a `callback_id` parameter of type `str` or `re.Pattern`. | ||
|
||
Shortcuts must be acknowledged with `ack()` to inform Slack that your app has received the event. | ||
Shortcuts must be acknowledged with `ack()` to inform Slack that your app has received the request. |
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 should update line 12 in this file to also use requests
instead of events
.
listen to incoming shortcut events
@stevengill I have made the changes you requested. Please take a look at it and let me know if this needs any changes. |
Codecov Report
@@ Coverage Diff @@
## main #415 +/- ##
=======================================
Coverage 91.36% 91.36%
=======================================
Files 167 167
Lines 5491 5491
=======================================
Hits 5017 5017
Misses 474 474 Continue to review full report at Codecov.
|
@stevengill This pull request now looks good to me. Do you have any comments on it? |
Will merge this pull request tomorrow if there is no other comment |
@RhnSharma Thanks a lot for making this pull request 👍 |
Fixes #412
Hi @seratch, how does this look?
Let me know if this needs any changes.
Thanks
Category (place an
x
in each of the[ ]
)slack_bolt.App
and/or its core componentsslack_bolt.async_app.AsyncApp
and/or its core componentsslack_bolt.adapter
/docs
Requirements (place an
x
in each[ ]
)Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you are agreeing to those rules.
./scripts/install_all_and_run_tests.sh
after making the changes.