Skip to content

Conversation

misl
Copy link
Contributor

@misl misl commented Oct 17, 2019

QuarkusMediatorConfigurationUtil.JandexGenericTypeAssignable.check() results in NotAssignable where Assignable is expected. Especially the following part: argumentClass.isAssignableFrom(target). This basically results in AmqpMessage.isAssignableFrom(Message) or MqttMessage.isAssignableFrom(Message). This should be the other way around Message.isAssignableFrom(AmqpMessage).

@geoand
Copy link
Contributor

geoand commented Oct 17, 2019

@cescoffier How do you want to deal with the testing part of this PR? Should we add a full blown integration test in the integration-tests directory?

@geoand
Copy link
Contributor

geoand commented Oct 18, 2019

Can you please rebase your PR onto the latest master in order to pick up a fix for CI?

Thanks

@cescoffier
Copy link
Member

We would need another PR with tests. Test would cover this but also other usages.

@cescoffier
Copy link
Member

So once rebased, and if CI is green, let's merge that and open another PR with tests.

Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

Thanks for this! Let's wait for CI and merge if all is green

@geoand geoand added this to the 0.26.0 milestone Oct 21, 2019
@geoand geoand added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Oct 21, 2019
@geoand
Copy link
Contributor

geoand commented Oct 21, 2019

@misl are you sure you rebased to the latest master? The native security tests are still failing and I am not seing that failure in other PRs.

Thanks

@gsmet gsmet merged commit fee254a into quarkusio:master Oct 21, 2019
@misl misl deleted the Fixing-4587 branch October 21, 2019 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

triage/waiting-for-ci Ready to merge when CI successfully finishes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants