Skip to content
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

[RESTEASY-2433] Ensure that Providers registered with @RegisterProvid… #2232

Merged
merged 1 commit into from
Dec 11, 2019

Conversation

geoand
Copy link
Contributor

@geoand geoand commented Nov 27, 2019

…er use the InjectorFactory.

Forms part of the solution for: quarkusio/quarkus#5752

@geoand
Copy link
Contributor Author

geoand commented Nov 27, 2019

@asoldano I'm not really sure how to add a test for this one.

Also I could have made the change to the specific method that registers providers only, but what I actually did seems conceptually more correct to me, but I might be wrong of course.

Also if possible we would like to get this fix in for Quarkus 1.1.0 which is due for mid December IIRC.

@marcinczeczko
Copy link

@geoand - are you going to address also lack of support for injection in ClientHeadersFactory (that can be registered via @RegisterClientHeaders) ? I found out that besides my issue quarkusio/quarkus#5752 there is another that asks for client headers too: quarkusio/quarkus/issues/5030

@geoand
Copy link
Contributor Author

geoand commented Nov 27, 2019

@marcinczeczko This PR won't take care of that case unfortunately. But looking at that issue, it's not really certain if we should fix it.
I mean I think we should, but it seems like the spec is unclear.

@gsmet
Copy link
Contributor

gsmet commented Nov 27, 2019

As for updating RESTEasy for Quarkus 1.1, the idea would be to have the upgrade PR on December 6th latest so that we can include it in the CR1 planned for December 9th.

@geoand
Copy link
Contributor Author

geoand commented Nov 27, 2019

@asoldano ^

Copy link
Member

@asoldano asoldano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the late feedback. I've been busy with some work that's possibly related to the changes here so wanted to have an idea of that first. Anyway, it looks like that won't be completed any time soon so I'm now reviewing this independently and I think the changes are acceptable.

@asoldano asoldano merged commit 637e5cb into resteasy:master Dec 11, 2019
@asoldano asoldano added the main label Dec 11, 2019
@geoand
Copy link
Contributor Author

geoand commented Dec 11, 2019

Thanks @asoldano !

@geoand geoand deleted the restclient-registerProvider branch December 11, 2019 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants