-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Register the StorkClientRequestFilter when we detect a URI starting with stork:// or storks:// #28296
Register the StorkClientRequestFilter when we detect a URI starting with stork:// or storks:// #28296
Conversation
Thanks for your pull request! The title of your pull request does not follow our editorial rules. Could you have a look?
|
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.
We should probably move this logic to build time as a next step, but it's fine for now to overcome the original issue
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.
The changes look good to me, however I think it can easily be covered by reusing this test: https://github.com/quarkusio/quarkus/blob/main/extensions/resteasy-reactive/rest-client-reactive/deployment/src/test/java/io/quarkus/rest/client/reactive/stork/StorkDevModeTest.java
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 don't know very well this part about filters so this LGTM.
Shouldn't we add a test also?
Looking to the CI fails
This comment has been minimized.
This comment has been minimized.
...tive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/impl/WebTargetImpl.java
Outdated
Show resolved
Hide resolved
Sorry got caught up by another urgent matter. I will have a look at the issues tomorrow morning. (And here goes my blog post...) |
This comment has been minimized.
This comment has been minimized.
Registers the StorkClientRequestFilter when we detect a URI starting with stork:// or storks://.
69ae399
to
1563b88
Compare
@Sgitario I added some tests. |
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!
Fix #28292