Closed GoogleCodeExporter closed 9 years ago
PS: my IDE tricked me. I'll submit a patch without white space changes on
monday.
Original comment by rafael....@gmail.com
on 19 Jul 2013 at 3:48
This time without my company's source code style sheet.
Original comment by rafael....@gmail.com
on 22 Jul 2013 at 7:15
Attachments:
Hi Rafael,
Thanks for submitting a patch. The second version is definitely nicer and
smaller ;-) There are still a few indentation problems, I think. But aside from
that, the code is simple enough to follow.
@Nate: What do you think about this proposal? Do you think we should include
something like this?
Original comment by romixlev
on 23 Jul 2013 at 1:38
Hey guys, I was wondering if there was any progress on that matter? I am
currently using a forked version of Kryo, but I would love to return to the
official distribution in the long run. Thanks for keeping me updated!
Original comment by rafael....@gmail.com
on 4 Aug 2013 at 8:26
@Nate: Any comments from your side about it?
-Leo
Original comment by romixlev
on 5 Aug 2013 at 11:00
Is it really so bad to subclass Kryo and override newDefaultSerializer?
Original comment by nathan.s...@gmail.com
on 5 Aug 2013 at 12:18
Well, relying on an overriden newDefaultSerializer method breaks the expected
behavior of the Kryo class. Of course, both ways do the job, but if you want to
offer a custom serializer to be used by others which might use their own Kryo
configurations, you either have to tell them explicitly to additionally
overriding the method of their Kryo instances or otherwise have runtime
exceptions thrown.
I consider this a hack and this is why I suggested the change. See this example
of a Spring aware serializer https://github.com/magro/kryo-serializers/pull/11
for why I think this matters.
Original comment by rafael....@gmail.com
on 5 Aug 2013 at 12:39
Ok. I think it makes sense. I'm fine with the patch being applied. Would you
like to do the honors, Leo? I'm absolutely buried in work. :(
PseudoSerializerFactory is an interesting class name. :)
Original comment by nathan.s...@gmail.com
on 22 Aug 2013 at 7:26
OK. I'll try to apply the patch tomorrow, run all our unit tests, and if
everything is fine I'll commit it.
Original comment by romixlev
on 22 Aug 2013 at 7:37
OK. I merged the patch into trunk.
@Raphael: If you could provide some Javadocs based on the trunk version
containing your changes, it would be nice.
Original comment by romixlev
on 23 Aug 2013 at 2:12
Ok, I will send some Javadocs the next days.
Original comment by rafael....@gmail.com
on 23 Aug 2013 at 3:34
And here it is.
Original comment by rafael....@gmail.com
on 25 Aug 2013 at 1:53
Attachments:
Thanks, Rafael. I applied your Javadoc patch. Should we close this issue now?
Original comment by romixlev
on 26 Aug 2013 at 6:25
From my side, there is nothing left to to. Thanks for the quick fix!
Original comment by rafael....@gmail.com
on 26 Aug 2013 at 6:38
Original comment by martin.grotzke
on 30 Sep 2013 at 9:38
Original issue reported on code.google.com by
rafael....@gmail.com
on 19 Jul 2013 at 2:09Attachments: