support for custom serializer #2
Conversation
|
@cyrilschumacher I agree this is a needed feature |
|
My company has been using this in production for months now. I just never got around to making a PR. Do you want more unit test coverage before this is merged? |
There was a problem hiding this comment.
Hello @danomatic and thanks for this pull request ;-) Can you look at my comments?
| @@ -68,6 +69,7 @@ | |||
| private final long ttl; | |||
|
|
|||
| private final Object lock = new Object(); | |||
| private Serializer serializer; | |||
There was a problem hiding this comment.
Can you add final keyword on this field?
|
|
||
| import java.io.IOException; | ||
|
|
||
| /** |
There was a problem hiding this comment.
Can you remove this Java documentation? or complete it?
| builder.withSerializer(new JavaSerializer()); | ||
| builder.withTTL(TTL); | ||
| builder.withFlushOnBoot(false); | ||
| builder.build(); |
There was a problem hiding this comment.
Can we add assertions on the built object to verify it has been built with the correct values?
| ); | ||
| builder.withTTL(TTL); | ||
| builder.withFlushOnBoot(false); | ||
| builder.build(); |
There was a problem hiding this comment.
Can we add assertions on the built object to verify it has been built with the correct values?
|
@danielprplt all the requested changes have been made |
|
This is really good. |
|
@danomatic Could you improve the coverage (if possible)? @danielprplt What do you think? |
|
@danomatic Could you resolve the conflicts present in your branch? I merged your pull request. ;-) |
Allow applications to provide their own serializer, because the Java serializer can be problematic.