-
Notifications
You must be signed in to change notification settings - Fork 40
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
Permit UUID by default #223
Conversation
…zation by default.
25c412a
to
59b067f
Compare
addClassPair(Float.class, "Float"); | ||
addClassPair(Float.class, "Float"); | ||
addClassPair(Float.class, "Float"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note the duplication here, this has been removed.
@@ -125,41 +128,44 @@ public Invocation deserializeInvocation(Reader reader) { | |||
addClassPair(boolean.class, "boolean"); | |||
addClassPair(char.class, "char"); | |||
|
|||
addClassPair(Byte.class, "Byte"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no need to duplicate the class simple name like this. Just get it from the class directly.
addClass(ChronoUnit.class); | ||
|
||
addClass(Transaction.class); | ||
addClass(TransactionContextPlaceholder.class); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That differs from addClassPair(TransactionContextPlaceholder.class, "TransactionContext");
, is it ok?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good spot! I went through them and failed on the last line. I will undo that - I don't have an interest in accidentally altering behaviour here - the old name might be persisted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. Reverted to the old name for this one.
Thanks @tsg21. Merged :) |
* Allow java.util.UUID as one of the types that are allowed for serialization by default. * Revert unintended behaviour change. (cherry picked from commit 7fdac33)
Make
DefaultInvocationSerializer
permitjava.util.UUID
Fixes #217