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

Fix NoSuchMethodException in runtime for Kotlin 1.4 #557

Merged
merged 1 commit into from
May 23, 2022

Conversation

Spikhalskiy
Copy link
Contributor

@Spikhalskiy Spikhalskiy commented Mar 29, 2022

Add an explicit language check before code checking for value classes

Closes #556

@cowtowncoder
Copy link
Member

WDYT @dinomite @viartemev ?

@Spikhalskiy
Copy link
Contributor Author

Spikhalskiy commented May 22, 2022

@cowtowncoder I made a snapshot build with this change and now basic things work fine with Kotlin 1.4 in my project. Can we merge it during the absence of maintainers?
By its nature (just if statement) this PR only affects Kotlin 1.4 which is not supported at all right now without it.
Let me know if I also should open the PR against 2.13 instead of 2.14.

@cowtowncoder cowtowncoder merged commit 3f259ba into FasterXML:2.14 May 23, 2022
@cowtowncoder
Copy link
Member

@Spikhalskiy np, I cherry-picked the fix into 2.13 branch.

One thing I forgot to ask for is CLA, would it be possible to get that done? Unless you have done it before
(I thought you had and merged but then realized that this might not be the case)
Document is at:

https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf

and the usual way is to print it, fill & sign, scan/photo, email pdf/png/jpeg to info at fasterxml dot com.
This only needs to be done once, for the first merged contribution: CLA is good for all future contributions.

@cowtowncoder
Copy link
Member

Aside from CLA, I think it'd be great to figure out a way to use Github Actions to verify build against multiple different Kotlin core versions. I think it might as simple as just overriding Maven property version.kotlin in CI; if I have time I can play with it. Or you or anyone else who has time and interest. While this would not allow local testing -- locally one can invoke mvn with same override tho? And maybe we could change build to... hmmh -- well, at least we can start by CI flagging breakages, and that would be run against PRs too.

@Spikhalskiy
Copy link
Contributor Author

Spikhalskiy commented May 23, 2022

@cowtowncoder
CLA. it's not my first contribution, so if nothing changed recently - it should be done already.

About verifying the build... this will actually not build under kotlin 1.4, because the methods that were added in kotlin 1.5 are used. But it works fine.
As an alternative, some super simple unit/integration test may be added that runs with kotlin 1.4 as a sanity check.
Or we can run a subset of tests that are applicable for kotlin 1.4 (most of them) under kotlin 1.4.

@Spikhalskiy Spikhalskiy deleted the 2.13 branch May 23, 2022 02:24
@cowtowncoder
Copy link
Member

Ok good point wrt building. Github action can definitely be split in a way to build with 1.5, then just run test(s) with other "minor" versions. That actually makes more sense, too, since our release will have to use specific official dependency and it is output of that which has to pass test.

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.

Broken Kotlin 1.4 support in 2.13.2
2 participants