-
Notifications
You must be signed in to change notification settings - Fork 273
feat: accept assistant.threads.setStatus method arguments from the assistant class #1371
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
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## ai-apps #1371 +/- ##
==========================================
Coverage ? 91.00%
==========================================
Files ? 222
Lines ? 7514
Branches ? 0
==========================================
Hits ? 6838
Misses ? 676
Partials ? 0 ☔ View full report in Codecov by Sentry. |
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.
Looking solid @zimeg! I'll test this out momentarily.
❓ Are you still planning to add some unit tests to this?
thread_ts=self.thread_ts, | ||
status=status, | ||
loading_messages=loading_messages, | ||
**kwargs, |
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.
priaise: The **kwargs
is a nice touch to add some future proofing for additional arguments. 🆗
@mwbrooks I was hoping to've found tests sooner! Thank you for reviewing this so fast though 🎁 ✨ Let's mark this as a draft for now, but I will aim for soon follow up. |
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.
✅ Beauty!
🧪 I've tested it with the sample app branch and things look good!
📚 Thanks for generating the docs! 🙇🏻
@mwbrooks Thank you! Let's merge this for upcoming releases in more testing and similar 🚢 💨 |
Summary
This PR accepts
assistant.thread.setStatus
method arguments from theAssistant
class to support additional arguments.Preview
The following is now supported when using the
Assistant
class:Testing
Use the changes of this PR and this sample app: slack-samples/bolt-python-assistant-template#12 🏁
Notes
Hoping to align a similar implementation for
bolt-js
too: slackapi/bolt-js#2660 👾Category
slack_bolt.App
and/or its core componentsslack_bolt.async_app.AsyncApp
and/or its core componentsRequirements
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.