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

Bump Jackson to 2.15.2 #33762

Closed
wants to merge 11 commits into from
Closed

Bump Jackson to 2.15.2 #33762

wants to merge 11 commits into from

Conversation

KaiSuchomel
Copy link
Contributor

In Jackson 2.15.2 the following Issue was fixed:
FasterXML/jackson-databind#3895

Added Test for Jackson 2.15.2 Issue.

@quarkus-bot quarkus-bot bot added area/dependencies Pull requests that update a dependency file area/resteasy-classic labels Jun 1, 2023
pom.xml Outdated
Comment on lines 54 to 56
<maven.compiler.target>17</maven.compiler.target>
<maven.compiler.source>17</maven.compiler.source>
<maven.compiler.release>17</maven.compiler.release>
Copy link
Contributor

Choose a reason for hiding this comment

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

We definitely don't want this :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed it to get my Test using "record" running. Is there another way to enable Java version with "record"??

Copy link
Contributor

Choose a reason for hiding this comment

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

I would say let's remove the test altogether as it's not a Quarkus issue, it's a Jackson issue so we should not be testing for it (unless we have some problematic integration point)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a plan to update the maven.compiler settings to a newer version than 11?
So far i will remove the Test.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a plan to update the maven.compiler settings to a newer version than 11?

No.

But for Java 17 specific things, we do have a dedicated integration test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, where can i find these dedicated Tests?

Copy link
Member

Choose a reason for hiding this comment

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

If it’s a Jackson bug, I don’t think we need a test in our code base.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, than next time we need to check again manually...

Copy link
Contributor

Choose a reason for hiding this comment

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

If we were to add such tests, we would need to add them for every feature under the sun, that just isn't practical.
What we do test extensively is our integration points with the various libraries and our workarounds for known problems.

The Jackson problem you linked to is neither - but of course we do want to bump to the latest Jackson version.

@geoand geoand changed the title Jackson 2.15.0 Issue with record Serialization with Getter Visibility Bump to Jackson 2.15.2 Jun 1, 2023
@geoand
Copy link
Contributor

geoand commented Jun 1, 2023

Thanks for the contribution!

Please rebase the PR onto main (so we can get rid of the merge commit) after removing the invalid change in the pom,xml file I pointed out above.

@geoand geoand changed the title Bump to Jackson 2.15.2 Bump Jackson to 2.15.2 Jun 1, 2023
@quarkus-bot quarkus-bot bot added the area/jackson Issues related to Jackson (JSON library) label Jun 1, 2023
@gsmet gsmet mentioned this pull request Jun 6, 2023
@gsmet
Copy link
Member

gsmet commented Jun 6, 2023

Thanks but this was incomplete. I created #33846 and kept your commit and authorship in there.

@gsmet gsmet closed this Jun 6, 2023
@quarkus-bot quarkus-bot bot added the triage/invalid This doesn't seem right label Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dependencies Pull requests that update a dependency file area/jackson Issues related to Jackson (JSON library) area/resteasy-classic triage/invalid This doesn't seem right
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants