Skip to content

Conversation

@seratch
Copy link
Contributor

@seratch seratch commented Apr 14, 2021

Summary

This pull request resolves #1000 by adding slack_sdk.rtm_v2.RTMClient as the default way to use the v2 RTM client. We still continue supporting slack_sdk.rtm.v2 as an alias for this module.

Category (place an x in each of the [ ])

  • slack_sdk.web.WebClient (sync/async) (Web API client)
  • slack_sdk.webhook.WebhookClient (sync/async) (Incoming Webhook, response_url sender)
  • slack_sdk.models (UI component builders)
  • slack_sdk.oauth (OAuth Flow Utilities)
  • slack_sdk.socket_mode (Socket Mode client)
  • slack_sdk.audit_logs (Audit Logs API client)
  • slack_sdk.scim (SCIM API client)
  • slack_sdk.rtm (RTM client)
  • slack_sdk.signature (Request Signature Verifier)
  • /docs-src (Documents, have you run ./docs.sh?)
  • /docs-src-v2 (Documents, have you run ./docs-v2.sh?)
  • /tutorial (PythOnBoardingBot tutorial)
  • tests/integration_tests (Automated tests for this library)

Requirements (place an x in each [ ])

  • 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 python setup.py validate after making the changes.

@seratch seratch added bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented rtm-client Version: 3x labels Apr 14, 2021
@seratch seratch added this to the 3.5.0 milestone Apr 14, 2021
@seratch seratch self-assigned this Apr 14, 2021
@codecov
Copy link

codecov bot commented Apr 14, 2021

Codecov Report

Merging #1001 (4f3faff) into main (aebf6db) will increase coverage by 0.03%.
The diff coverage is 52.08%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1001      +/-   ##
==========================================
+ Coverage   86.58%   86.62%   +0.03%     
==========================================
  Files          94       95       +1     
  Lines        8747     8748       +1     
==========================================
+ Hits         7574     7578       +4     
+ Misses       1173     1170       -3     
Impacted Files Coverage Δ
slack_sdk/rtm_v2/__init__.py 51.88% <51.88%> (ø)
slack_sdk/rtm/v2/__init__.py 100.00% <100.00%> (+48.11%) ⬆️
slack_sdk/socket_mode/client.py 76.99% <0.00%> (+2.65%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aebf6db...4f3faff. Read the comment docs.

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.

Looks good and nice to see a few rocket ship reactions 🚀

This is part of v3.5.0 because it's a breaking change from due to changing the import path?

@seratch
Copy link
Contributor Author

seratch commented Apr 14, 2021

@mwbrooks

This is part of v3.5.0 because it's a breaking change from due to changing the import path?

Let me clarify this a bit more.

This is not a breaking change to any existing apps depending on slack_sdk.rtm.v2 package. As long as it works for the apps, they don't need to change imports at all. The package still exists and works as like before.

The new source package slack_sdk.rtm_v2 enables new developers (or the ones who are currently wanting to remove aiohttp from their apps) not to have aiohttp only for the reason where slack_sdk.rtm.v2 package requires it.

@seratch seratch merged commit 134813c into slackapi:main Apr 14, 2021
@seratch seratch deleted the issue-1000-rtm_v2 branch April 14, 2021 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented rtm-client Version: 3x

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RTMClient v2 still requires aiohttp installed (even though it's unused)

2 participants