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

Use box-impl function as factory for kotlin value class #4067

Closed
wants to merge 1 commit into from

Conversation

wplong11
Copy link

@wplong11 wplong11 commented Aug 3, 2023

@wplong11 wplong11 force-pushed the support-value-class branch 4 times, most recently from 8fc9177 to 9ccc5b7 Compare August 3, 2023 17:55
boolean isKotlinValueClassFactory = m.isSynthetic() && m.getName().equals("box-impl");
if (isKotlinValueClassFactory) {
return true;
}
Copy link
Member

Choose a reason for hiding this comment

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

Though it may be just a variable name, supporting specific use case for down stream modules such should be cautiously investigated.

Copy link
Author

Choose a reason for hiding this comment

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

This is working PoC. Implementation details should be polished
#4066 (comment)

@JooHyukKim
Copy link
Member

Interesting approach though. @wplong11 Have you tried building this project locally with new changes? How are the test cases affected?

@pjfanning
Copy link
Member

Interesting approach though. @wplong11 Have you tried building this project locally with new changes? How are the test cases affected?

@JooHyukKim we are asking that @wplong11 look at https://github.com/ProjectMapK/jackson-module-kogera/ and work with that project to get value classes working

Ideally, jackson-databind shouldn't have Kotlin specific code.

@JooHyukKim
Copy link
Member

@pjfanning Oh okay, so this PR relates to #4066 (comment). Thanks! 👍🏻

@wplong11
Copy link
Author

wplong11 commented Aug 4, 2023

Included in PR #4066

@wplong11 wplong11 closed this Aug 4, 2023
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.

3 participants