-
Notifications
You must be signed in to change notification settings - Fork 216
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
Add Jakarta EE compatible Socket Mode client ref: #919 #1352
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1352 +/- ##
============================================
- Coverage 74.99% 74.82% -0.18%
- Complexity 4203 4274 +71
============================================
Files 453 457 +4
Lines 12975 13276 +301
Branches 1342 1369 +27
============================================
+ Hits 9731 9934 +203
- Misses 2462 2544 +82
- Partials 782 798 +16 ☔ 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.
This looks good to me 💯 the changes seem to be mainly be a copy of existing code 👍
I'm assuming this code has been stable for a while, maintaining this type of duplication should not be a problem
...t/src/main/java/com/slack/api/jakarta_socket_mode/impl/JakartaSocketModeClientTyrusImpl.java
Outdated
Show resolved
Hide resolved
@@ -0,0 +1,435 @@ | |||
package com.slack.api.jakarta_socket_mode.impl; |
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.
I see this is almost an exact copy of SocketModeClientTyrusImpl.java
, which is production worthy 🧠
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.
Yeah, you're right!
…akarta_socket_mode/impl/JakartaSocketModeClientTyrusImpl.java Co-authored-by: William Bergamin <william.bergamin.coen@gmail.com>
This pull request adds two new optional modules:
The currently available Socket Mode client's default implementation uses tyrus-standalone-client 1.x, which is compatible with javax.websocket-api APIs. The Jakarta EE version of this interface is the jakarta.websocket-client-api APIs, and tyrus-standalone-client 2.x is compatible with it. Since it's not feasible to have both tyrus-standalone-client 1.x and 2.x in the same module's dependencies, I have added a new module named slack-jakarta-socket-mode-client. See #919 for more details.
Developers can initialize the Jakarta-compatible SocketModeClient this way:
In the same way, I’ve added a new Jakarta-compatible module, which is equivalent to bolt-socket-mode. Here is the demo code. As you can see, just replace the dependency and imports in the code:
The reason behind this enhancement is that many Java-focused companies are planning to eliminate the legacy javax.* dependencies from their project settings. I don't think the short-term risk of having a javax.websocket dependency is significant, but it seems it's about time to provide an option for migration on the developers' side.
Category (place an
x
in each of the[ ]
)Requirements
Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you agree to those rules.