Skip to content

Conversation

@iqnev
Copy link

@iqnev iqnev commented Jul 18, 2024

No description provided.

@ivelinyanev-ship-cars ivelinyanev-ship-cars force-pushed the main branch 2 times, most recently from 82ccaa3 to 16e838e Compare July 19, 2024 11:45
@iqnev
Copy link
Author

iqnev commented Jul 19, 2024

Hi @SlyngDK I have provided a potential fix for this issue #169

Could you please review it and provide your feedback?

@iqnev iqnev changed the title Add the new configuration proprty[enabledJavaTimeModule] Add the new configuration property[enabledJavaTimeModule] Jul 19, 2024
@SlyngDK
Copy link
Contributor

SlyngDK commented Jul 20, 2024

@iqnev I don't like the hard coded register approach.
But I can accept some sort of configuration, for which jackson modules to register.

And the issue with modules is not auto registered is only an issue with native?
Is there an other way to provide/detect the modules in native build?

@iqnev
Copy link
Author

iqnev commented Jul 21, 2024

@iqnev I don't like the hard coded register approach. But I can accept some sort of configuration, for which jackson modules to register.

And the issue with modules is not auto registered is only an issue with native? Is there an other way to provide/detect the modules in native build?

@SlyngDK Actually, the current PR offers a fully configurable approach. The issue arises specifically with native mode. I had a brief conversation with @geoand , who mentioned that providing a default bean with JavaTimeModule() within the quarkus-logging-json extension could lead to a conflict with the quarkus-jackson extension

@ls-bit
Copy link

ls-bit commented Jun 4, 2025

Could it be an option to implement same behaviour as in the quarkus-jackson extension which is to autoregister modules like JavaTimeModule depending on if they are found on the classpath? See https://github.com/quarkusio/quarkus/blob/3.23/extensions/jackson/deployment/src/main/java/io/quarkus/jackson/deployment/JacksonProcessor.java#L358

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants