Upgrade RESTEasy from 4.6.1.Final to 4.7.0.Final #18623
Conversation
|
Thanks for your pull request! The title of your pull request does not follow our editorial rules. Could you have a look?
|
quarkus-bot
bot
added
the
area/dependencies
jamezp
changed the title
bom/application/pom.xml
Outdated
| <version>${resteasy.version}</version> | ||
| <type>pom</type> | ||
| <scope>import</scope> | ||
| </dependency> |
There was a problem hiding this comment.
Why did you move the BOM? The BOM order is important so I wouldn't move it except if we have a good reason.
There was a problem hiding this comment.
There was a conflict with the version of Vert.x the RESTEasy BOM was importing. It was causing compile errors. I just moved it below the Vert.x import.
There was a problem hiding this comment.
Why is the RESTEasy BOM including a version of Vert.x? This doesn't seem like a normal behavior.
There was a problem hiding this comment.
RESTEasy has a Vert.x module (that is different to the Vert.x support we provide).
There was a problem hiding this comment.
Yes sure but it shouldn't include a version for Vert.x artifacts.
I put the BOM back where it was and I don't have any compilation issue.
I'll push to a branch in my fork to see what CI has to say.
There was a problem hiding this comment.
Also I made a diff of the effective POM and there's no difference except for the position. BOMs are a sensible matter so I prefer not changing it if we can avoid it.
|
This workflow status is outdated as a new workflow run has been triggered. Failing Jobs - Building 91e2237
Full information is available in the Build summary check run. Test Failures⚙️ JVM Tests - JDK 11 Windows #📦 extensions/arc/deployment✖ |
I'm not too sure what could be done to sort it out short of using the same version of Vert.x in Quarkus and RESTEasy. Even then it just hides it until it diverges again. |
|
@jamezp the RESTEasy BOM doesn't enforce any Vert.x version AFAICS so it shouldn't change anything. And indeed, I moved it back and it doesn't change anything. I pushed the old order to CI, let's see how it goes. |
|
@gsmet I think that's because of this fix https://github.com/resteasy/Resteasy/pull/2856/files#diff-804f7d09a86dc42c94770466e216189fa8816942d41764cb4d31a69b2013f37dR30-R31. If you use 4.7.0.Beta1 which doesn't include that and uses Vert.x 4.0.3 you'll see a compile error in I will admittedly do not understand how the Anyway whatever you prefer. If an upgrade to Vert.x breaks it in the future we'll know how to fix it :) |
bom/application/pom.xml
Outdated
| @@ -256,6 +256,7 @@ | |||
|
|
|||
| <!-- RESTEasy dependencies, imported as a BOM --> | |||
| <!-- As RESTEasy also imports Jackson, we need to import its BOM after the Jackson BOM --> | |||
| <!-- As RESTEasy also imports Vert.x transitively, we need to import its BOM after the Vert.x BOM --> | |||
There was a problem hiding this comment.
This could be removed now since we didn't move the import.
There was a problem hiding this comment.
Yeah, I'll clean everything if I can get CI to pass. If not I'll dig a bit more.
gsmet
added
the
triage/waiting-for-ci
|
This workflow status is outdated as a new workflow run has been triggered. 🚫 This workflow run has been cancelled. Failing Jobs - Building c244733
|
|
CI is happy, so let's merge. I'll follow up with re-enabling those rest-client TCK tests that had to be disabled for Java 16. |
quarkus-bot
bot
removed
the
triage/waiting-for-ci
Resolves #18622