Skip to content

Conversation

@zimeg
Copy link
Member

@zimeg zimeg commented Sep 18, 2025

Summary

This PR adds an optional loading_messages attribute to the assistant.threads.setStatus method.

Testing

WIP.

Category

  • slack_sdk.web.WebClient (sync/async) (Web API client)

Requirements

  • I've read and understood the Contributing Guidelines and have done my best effort to follow them.
  • I've read and agree to the Code of Conduct.
  • I've run python3 -m venv .venv && source .venv/bin/activate && ./scripts/run_validation.sh after making the changes.

@zimeg zimeg self-assigned this Sep 18, 2025
@zimeg zimeg added enhancement M-T: A feature request for new functionality semver:minor web-client Version: 3x labels Sep 18, 2025
@codecov
Copy link

codecov bot commented Sep 18, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (ai-apps@f60e37c). Learn more about missing BASE report.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             ai-apps    #1746   +/-   ##
==========================================
  Coverage           ?   84.94%           
==========================================
  Files              ?      113           
  Lines              ?    12916           
  Branches           ?        0           
==========================================
  Hits               ?    10972           
  Misses             ?     1944           
  Partials           ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@srtaalej srtaalej left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Member

@mwbrooks mwbrooks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Everything looks good on my side! Great work, again, @zimeg! 🙇🏻

🧪 We discussed offline to add the tests later, since this feature isn't fully complete. We have a task, so tests will be added before this is brought into main.

@srtaalej feel empowered to hit that Approve button if you're confident in the code changes 😄 @zimeg deserves multiple approvals from time-to-time 😉

loading_messages: Optional[List[str]] = None,
**kwargs,
) -> AsyncSlackResponse:
"""Revokes a token.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

praise: Oof, thanks for catching this typo (and the ones following).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mwbrooks Thank you! I was debating opening a separate PR but am wanting to do a much more thorough pass for such 📚

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries, since it's internal cleanup and the PR is small, it didn't distract from the core changes in the PR 😄 You made the right call!

@zimeg zimeg changed the base branch from main to ai-apps September 19, 2025 04:04
@zimeg
Copy link
Member Author

zimeg commented Sep 19, 2025

@srtaalej @mwbrooks I appreciate the reviews always! Let's get this merged to match method definitions in ongoing development 🤖

@zimeg zimeg merged commit 4a3c0ed into ai-apps Sep 19, 2025
12 checks passed
@zimeg zimeg deleted the zimeg-feat-web-api-assistant-thread-loading-messages branch September 19, 2025 04:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement M-T: A feature request for new functionality semver:minor Version: 3x web-client

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants