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

Upgrade to Jandex 3.0.0 #27538

Merged
merged 2 commits into from
Sep 16, 2022
Merged

Upgrade to Jandex 3.0.0 #27538

merged 2 commits into from
Sep 16, 2022

Conversation

Ladicek
Copy link
Contributor

@Ladicek Ladicek commented Aug 26, 2022

Submitting as draft because this needs discussion on release coordination (I guess).

Currently depends on SNAPSHOT of SmallRye GraphQL.

@quarkus-bot quarkus-bot bot added area/arc Issue related to ARC (dependency injection) area/core area/dependencies Pull requests that update a dependency file area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/documentation area/gradle Gradle area/graphql area/hibernate-orm Hibernate ORM area/hibernate-search Hibernate Search / Elasticsearch area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure area/maven area/mongodb area/panache area/persistence OBSOLETE, DO NOT USE area/picocli area/platform Issues related to definition and interaction with Quarkus Platform area/rest area/smallrye area/spring Issues relating to the Spring integration area/testing area/vertx labels Aug 26, 2022
@quarkus-bot
Copy link

quarkus-bot bot commented Aug 26, 2022

/cc @Sanne, @gsmet, @yrodiere

@gastaldi
Copy link
Contributor

@Ladicek don't forget #27520

@gastaldi
Copy link
Contributor

Do you think it's interesting to add the org.jboss.jandex as a banned dependency in the enforcer plugin (build-parent)?

@Ladicek
Copy link
Contributor Author

Ladicek commented Aug 29, 2022

Thanks, fixed independent-projects/bootstrap. The Enforcer plugin is already configured to ban org.jboss:jandex in this PR -- it is a recommendation I make in the Jandex 3.0.0 release announcement, so I'm well aware of it :-)

@Ladicek Ladicek force-pushed the jandex3 branch 3 times, most recently from 0f0266c to 185f4f7 Compare August 30, 2022 10:16
@Ladicek Ladicek force-pushed the jandex3 branch 2 times, most recently from 7f5e9f2 to 661c3c5 Compare September 13, 2022 15:20
@gsmet
Copy link
Member

gsmet commented Sep 14, 2022

@jmartisk @Ladicek main is now 2.14, so we can consider merging this when it's ready. IIRC, we need SmallRye GraphQL releases.

@Ladicek
Copy link
Contributor Author

Ladicek commented Sep 14, 2022

Correct, still waiting for SmallRye GraphQL 1.8.0.

@jmartisk
Copy link
Contributor

Ok, I'll go for the release now

@gsmet
Copy link
Member

gsmet commented Sep 14, 2022

@jmartisk I think you have that in mind but since you used the singular: we also need a Jakarta release.

@jmartisk
Copy link
Contributor

SmallRye GraphQL 2.0.0.RC9 and 1.8.0 are in Central.

@@ -585,7 +585,7 @@ recipeList:
newValue: 4.0.0-RC1
Copy link
Contributor

@jmartisk jmartisk Sep 15, 2022

Choose a reason for hiding this comment

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

Please update to 4.0.0 (SR GraphQL 2.0.0.RC9 depends on it)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That should be a different PR I guess?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah if GraphQL depends on it, then we can do that here as well, I think.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know, depends how hard we try to keep dependencies properly aligned :)

@Ladicek Ladicek marked this pull request as ready for review September 15, 2022 15:39
@Ladicek
Copy link
Contributor Author

Ladicek commented Sep 15, 2022

OK, marking as ready for review. The 1st commit deserves a review from @mkouba (though this change won't be necessary when I release Jandex 3.0.1, which should restore structural Type equality).

@Ladicek
Copy link
Contributor Author

Ladicek commented Sep 15, 2022

Note that the "quickstarts compilation" check in CI will fail due to:

That's all I'm currently aware of.

@gsmet gsmet requested a review from mkouba September 15, 2022 15:51
@gsmet
Copy link
Member

gsmet commented Sep 15, 2022

@mkouba can you have a look at the first commit? I'd like to get this merged ASAP. Thanks!

@gsmet gsmet dismissed their stale review September 15, 2022 17:14

Dismissed.

@quarkus-bot
Copy link

quarkus-bot bot commented Sep 15, 2022

Failing Jobs - Building fbc44b8

Status Name Step Failures Logs Raw logs
✔️ JVM Tests - JDK 11
JVM Tests - JDK 17 Build Failures Logs Raw logs
✔️ JVM Tests - JDK 18
Quickstarts Compilation - JDK 17 Compile Quickstarts Failures Logs Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 17 #

- Failing: extensions/resteasy-classic/resteasy/deployment 
! Skipped: extensions/agroal/deployment extensions/avro/deployment extensions/cache/deployment and 304 more

📦 extensions/resteasy-classic/resteasy/deployment

io.quarkus.resteasy.test.ReadTimeoutTestCase.testReadTimesOut line 45 - More details - Source on GitHub

org.opentest4j.AssertionFailedError: expected: <false> but was: <true>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)

⚙️ Quickstarts Compilation - JDK 17 #

- Failing: optaplanner-quickstart reactive-messaging-http-quickstart reactive-messaging-websockets-quickstart 

📦 optaplanner-quickstart

Failed to execute goal io.quarkus:quarkus-maven-plugin:999-SNAPSHOT:build (default) on project optaplanner-quickstart: Failed to build quarkus application

📦 reactive-messaging-http-quickstart

Failed to execute goal io.quarkus:quarkus-maven-plugin:999-SNAPSHOT:build (default) on project reactive-messaging-http-quickstart: Failed to build quarkus application

📦 reactive-messaging-websockets-quickstart

Failed to execute goal io.quarkus:quarkus-maven-plugin:999-SNAPSHOT:build (default) on project reactive-messaging-websockets-quickstart: Failed to build quarkus application

@mkouba
Copy link
Contributor

mkouba commented Sep 16, 2022

OK, marking as ready for review. The 1st commit deserves a review from @mkouba (though this change won't be necessary when I release Jandex 3.0.1, which should restore structural Type equality).

The first commit looks good. If there are problems we can revert the change once Jandex 3.0.1 is released.

Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

Let's get this in! Thanks for your work on this @Ladicek !

@gsmet gsmet merged commit e2511eb into quarkusio:main Sep 16, 2022
@quarkus-bot quarkus-bot bot added this to the 2.14 - main milestone Sep 16, 2022
@Ladicek Ladicek deleted the jandex3 branch September 16, 2022 08:37
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/core area/dependencies Pull requests that update a dependency file area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/documentation area/gradle Gradle area/graphql area/hibernate-orm Hibernate ORM area/hibernate-search Hibernate Search / Elasticsearch area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure area/jakarta area/maven area/mongodb area/panache area/persistence OBSOLETE, DO NOT USE area/picocli area/platform Issues related to definition and interaction with Quarkus Platform area/rest area/smallrye area/spring Issues relating to the Spring integration area/testing area/vertx release/noteworthy-feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants