-
Notifications
You must be signed in to change notification settings - Fork 566
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
WebSocket Extensions #1934
WebSocket Extensions #1934
Conversation
/oca-checked |
/trigger |
This is a problem. What do you think @barchetta? |
I agree that this PR should stay in WIP for now because:
So this PR might need to stay in the wings until we make some decisions concerning moving to Jakarta EE 9. |
It appears Checkstyle doesn't like the ordering too:
Would this have to change to |
Good question. This is debatable :D ; I'd say it should be before |
e5126bd
to
0ab85ae
Compare
It appears 1.17 has both the changes I submitted and still using |
Any idea how to re-trigger CI? |
/trigger |
0ab85ae
to
5e54abc
Compare
/trigger |
I'm hoping this will be merged soon but the CI never seems to trigger when I push. I tried using |
Basically accounts that do not have access to the repository are not "trusted" and their pull requests need to be manually triggered by "trusted" accounts. |
/trigger |
you have checkstyle errors, you can run the check locally: |
Thanks @romain-grecourt for the info. I've updated the MR with the checkstyle updates. |
/trigger |
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.
LGTM
@dansiviter we need to do some checks for the Tyrus version change before we merge this. |
@dansiviter thanks for the contribution and apologies this took so long to merge! |
Few notes:
org.glassfish.tyrus.ext.extension.deflate.PerMessageDeflateExtension
) that are in archives that are not 'bean archives' are not added as CDI ignores them. Is there a mechanism to override this or an acceptable limitation forcing workarounds? I thought about using synthetic bean archives, but the classpath would still need to be scanned to do this and not aware of any mechanism for this.The only build available with the fixes required in Tyrus is 2.0.0+ which also mandatesNo longer an issue,jakarta.websocket.*
packages which means quite a few more changes.The both Tyrus and WebSocket API are not at release versions yet. I'm assuming this would prevent merging.No longer an issue.