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

Avoid warning about indexing primitive types #15374

Merged
merged 1 commit into from Mar 2, 2021

Conversation

@geoand geoand requested a review from mkouba March 1, 2021 12:23
@quarkus-bot quarkus-bot bot added the area/arc Issue related to ARC (dependency injection) label Mar 1, 2021
@mkouba
Copy link
Contributor

mkouba commented Mar 1, 2021

So in general I believe that an extension should not even try to index a primitive type. It's easy to check a org.jboss.jandex.Type or even DotName and skip indexing for a primitive type.

Copy link
Contributor

@gastaldi gastaldi left a comment

Choose a reason for hiding this comment

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

That makes sense

@geoand
Copy link
Contributor Author

geoand commented Mar 1, 2021

So in general I believe that an extension should not even try to index a primitive type. It's easy to check a org.jboss.jandex.Type or even DotName and skip indexing for a primitive type.

I completely agree. The issue is that ATM, I don't know which extension is causing the problem

@mkouba
Copy link
Contributor

mkouba commented Mar 1, 2021

So in general I believe that an extension should not even try to index a primitive type. It's easy to check a org.jboss.jandex.Type or even DotName and skip indexing for a primitive type.

I completely agree. The issue is that ATM, I don't know which extension is causing the problem

Ok. Pls move the logic into the io.quarkus.arc.processor.Types.

CC @manovotn

@geoand
Copy link
Contributor Author

geoand commented Mar 1, 2021

Done

@quarkus-bot quarkus-bot bot added the area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins label Mar 1, 2021
@gastaldi
Copy link
Contributor

gastaldi commented Mar 1, 2021

So in general I believe that an extension should not even try to index a primitive type. It's easy to check a org.jboss.jandex.Type or even DotName and skip indexing for a primitive type.

I completely agree. The issue is that ATM, I don't know which extension is causing the problem

In the zulip chat it is mentioned that the error goes away when the openapi extension is removed, maybe that's a clue

@geoand
Copy link
Contributor Author

geoand commented Mar 1, 2021

So in general I believe that an extension should not even try to index a primitive type. It's easy to check a org.jboss.jandex.Type or even DotName and skip indexing for a primitive type.

I completely agree. The issue is that ATM, I don't know which extension is causing the problem

In the zulip chat it is mentioned that the error goes away when the openapi extension is removed, maybe that's a clue

Yeah, I took a quick look but I don't see anywhere in Quarkus where that is used.
My guess is that smallrye-openapi uses the index in its code and does not check for primitives

Copy link
Contributor

@manovotn manovotn left a comment

Choose a reason for hiding this comment

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

Looks good, this should do the trick.

@gastaldi
Copy link
Contributor

gastaldi commented Mar 1, 2021

So in general I believe that an extension should not even try to index a primitive type. It's easy to check a org.jboss.jandex.Type or even DotName and skip indexing for a primitive type.

I completely agree. The issue is that ATM, I don't know which extension is causing the problem

In the zulip chat it is mentioned that the error goes away when the openapi extension is removed, maybe that's a clue

Yeah, I took a quick look but I don't see anywhere in Quarkus where that is used.
My guess is that smallrye-openapi uses the index in its code and does not check for primitives

Maybe @phillip-kruger knows?

@geoand geoand added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Mar 1, 2021
@gsmet
Copy link
Member

gsmet commented Mar 1, 2021

Rebased to get a CI fix.

@geoand
Copy link
Contributor Author

geoand commented Mar 1, 2021

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/arc Issue related to ARC (dependency injection) area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins triage/waiting-for-ci Ready to merge when CI successfully finishes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants