-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Refactor async feign #1755
Refactor async feign #1755
Conversation
11413f0
to
81646ac
Compare
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 will merge as is, cause I really liked this change.
There is a chance it will break binary for people out there, but, it's an experimental API.
I think I will bump feign version as well to make clear it's breaking something
@@ -44,10 +45,17 @@ | |||
* be done (for example, creating and submitting a task to an {@link ExecutorService}). | |||
*/ | |||
@Experimental | |||
public abstract class AsyncFeign<C> extends Feign { |
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.
Loved this.
I was hating myself for having asyncBuilder
but couldn't take the time to fix it, amazing job, thanks
* Add MethodInfoResolver to customize MethodInfo creation logic * Add methodInfoResolver setter to AsyncBuilder * Refactor CoroutineFeign to use AsyncFeignBuilder instead of FeignBuilder * Deprecate CoroutineFeign.coBuilder * Change AsyncFeign to not inherit Feign * Deprecate AsyncFeign.asyncBuilder * Refactor AsyncBuilder to build Feign directly Co-authored-by: Marvin Froeder <velo@users.noreply.github.com>
* Add MethodInfoResolver to customize MethodInfo creation logic * Add methodInfoResolver setter to AsyncBuilder * Refactor CoroutineFeign to use AsyncFeignBuilder instead of FeignBuilder * Deprecate CoroutineFeign.coBuilder * Change AsyncFeign to not inherit Feign * Deprecate AsyncFeign.asyncBuilder * Refactor AsyncBuilder to build Feign directly Co-authored-by: Marvin Froeder <velo@users.noreply.github.com>
Contains #1754 PR commits
If you want to see the final result, see PR #1757
TODO