Use recursively determined fieldSerializer when looking up field update serializers. #99
Conversation
|
Can one of the admins verify this patch? |
|
Thanks @tbartley! Could you add a test for this fix before we merge? |
|
No problem. Will do. Sent from my iPhone On 17 Mar 2015, at 8:04 am, Ben McCann <notifications@github.commailto:notifications@github.com> wrote: Thanks @tbartleyhttps://github.com/tbartley! Could you add a test for this fix before we merge? Reply to this email directly or view it on GitHubhttps://github.com//pull/99#issuecomment-81961785. |
|
Unit test is now there. |
| } | ||
| public String _id = "id"; | ||
| public Inner inner = new Inner(); | ||
| public long timestamp; |
There was a problem hiding this comment.
duplicate space after long
|
Thanks for the comments @benmccann - how does that look now? |
|
@tbartley thanks! this PR is looking really good! a couple last minor comments... |
|
|
||
| Calendar cal = Calendar.getInstance(); | ||
| cal.set(0, 0, 0, 0, 0, 10); | ||
| Date d_1 = cal.getTime(); |
There was a problem hiding this comment.
_ is kind of uncommon in Java naming. I'd probably just call them d1 and d2
|
Thanks again. Went with 'original' oveer 'obj' since that's a clearer meaning and makes sense in the comparison. And I've removed the underscores. |
|
|
||
| Calendar cal = Calendar.getInstance(); | ||
| cal.set(0, 0, 0, 0, 0, 10); | ||
| Date d1 = cal.getTime(); |
There was a problem hiding this comment.
What if you just did:
Date d1 = new Date(10_000l);
It doesn't really matter what the date is and that'd be fewer lines of code, which I think makes the test more readable
There was a problem hiding this comment.
Agreed - something made me think there wasn't a Date(long) constructor.
|
Looks great! Sorry, just one last comment. And also, do you mind squashing the commits? |
tbartley
force-pushed
the
bug/98-class-cast-exception-on-update-serialization
branch
from
147904d
to
b11ea06
Compare
|
Done. |
…pdate-serialization Use recursively determined fieldSerializer when looking up field update serializers.
tbartley
deleted the
bug/98-class-cast-exception-on-update-serialization
branch
Use recursively determined fieldSerializer when looking up field update serializers. Closes #98.