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

Add support for Guava's ReverseList. #55

Merged
merged 2 commits into from
Oct 4, 2016

Conversation

amitsela
Copy link
Contributor

@amitsela amitsela commented Jul 25, 2016

Supporting Guava's ReverseList (and RandomAccessReverseList).
First time writing a Kryo serializer, appreciate the feedback.
Thanks!


This change is Reviewable

@amitsela
Copy link
Contributor Author

R: @magro

@coveralls
Copy link

coveralls commented Jul 25, 2016

Coverage Status

Coverage increased (+0.4%) to 82.488% when pulling 7eaa02f on amitsela:reverse-list-serializer into 9a4f71e on magro:master.

@coveralls
Copy link

coveralls commented Jul 25, 2016

Coverage Status

Coverage increased (+0.4%) to 82.488% when pulling 26ebad4 on amitsela:reverse-list-serializer into 9a4f71e on magro:master.

@coveralls
Copy link

coveralls commented Jul 25, 2016

Coverage Status

Coverage increased (+0.4%) to 82.488% when pulling 4ad8f5a on amitsela:reverse-list-serializer into 9a4f71e on magro:master.

@coveralls
Copy link

coveralls commented Jul 25, 2016

Coverage Status

Coverage increased (+0.4%) to 82.488% when pulling 5db1485 on amitsela:reverse-list-serializer into 9a4f71e on magro:master.

@magro magro merged commit 7c09d75 into magro:master Oct 4, 2016
@amitsela amitsela deleted the reverse-list-serializer branch October 4, 2016 22:11
magro pushed a commit that referenced this pull request Oct 4, 2016
@magro
Copy link
Owner

magro commented Oct 4, 2016

Thanks for the PR, I just released it with 0.39.

@amitsela
Copy link
Contributor Author

amitsela commented Oct 4, 2016

Great! happy to hear that.

@magro
Copy link
Owner

magro commented Oct 4, 2016

Btw, I also just updated the kryo dependency to 4.0.0 and released kryo-serializers 0.40 with this.

@amitsela
Copy link
Contributor Author

amitsela commented Oct 4, 2016

I'm mostly using Kryo with Spark so I assume that might not play nicely together 😉.

@magro
Copy link
Owner

magro commented Oct 4, 2016

Ah ok, might take a bit, as long as you cannot change the used kryo version on your own...

@amitsela
Copy link
Contributor Author

@magro quick question - can I use version 0.39 with Twitter chill-0.5.0 (depends on kryo-2.21) ? or should I use the latest 0.2x ? thanks.

@magro
Copy link
Owner

magro commented Oct 25, 2016

@amitsela Hmm, not sure, you could give it a try. If course you'd have to exclude the kryo dependency from kryo-serializers to stay with kryo 2.21.

@amitsela
Copy link
Contributor Author

amitsela commented Oct 25, 2016

@magro Yeah, did that, just worried about backward compatibility, though I'm only using kryo-serializers for Guava immutables and Java's UnmodifiableCollection:

UnmodifiableCollectionsSerializer.registerSerializers(kryo);
// Guava
ImmutableListSerializer.registerSerializers(kryo);
ImmutableSetSerializer.registerSerializers(kryo);
ImmutableMapSerializer.registerSerializers(kryo);
ImmutableMultimapSerializer.registerSerializers(kryo);
ReverseListSerializer.registerSerializers(kryo);

@magro
Copy link
Owner

magro commented Oct 25, 2016

@amitsela So does it work?

@amitsela
Copy link
Contributor Author

amitsela commented Oct 25, 2016

@magro Seems to... I know for sure that UnmodifiableCollectionsSerializer, and ReverseListSerializer work.

@magro
Copy link
Owner

magro commented Oct 25, 2016

Great!

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