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

Make Closure public, moving it to ClosureSerializer #415

Merged
merged 2 commits into from
Mar 30, 2016

Conversation

magro
Copy link
Collaborator

@magro magro commented Mar 29, 2016

Closure is moved to ClosureSerializer because they're tightly coupled and this makes the connection more obvious.

This also renames Kryo.isClousre to isClosure, assuming that it was not used until now it's ignored regarding binary compatibility.

A test for closure serialization is added together with the required maven foo for compiler settings.

Fixes #299, refs #215

Closure is moved to ClosureSerializer because it's tightly coupled
and this makes the connection more obvious.

This also renames `Kryo.isClousre` to `isClosure`, assuming that it
was not used until now it's ignored regarding binary compatibility.

Fixes EsotericSoftware#299, refs EsotericSoftware#215
@johnynek
Copy link

👍 we need this for twitter/chill#252

For this test the compiler source/target level for tests must be set to 1.8.
This is done via the "java8" profile which is activated if built with
java 8+. Java8*Tests are excluded by default from tests compilation, this
profile removes this exclude. Because overriding the compiler plugin in the
profile did not really work build properties are used.

To deactivate the java8 profile (e.g. to compile tests for jdks < 1.8)
mvn can be invoked with `-P !java8` (or `-P \!java8` on linux).

Refs EsotericSoftware#299
@magro
Copy link
Collaborator Author

magro commented Mar 30, 2016

@romix WDYT?

@romix
Copy link
Collaborator

romix commented Mar 30, 2016

No objections from me.

@magro
Copy link
Collaborator Author

magro commented Mar 30, 2016

Thanks for the review!

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

Successfully merging this pull request may close these issues.

3 participants