Skip to content

Big start into ClassAdapter.#19

Merged
swankjesse merged 1 commit intomasterfrom
jwilson_0322_classadapter
Mar 23, 2015
Merged

Big start into ClassAdapter.#19
swankjesse merged 1 commit intomasterfrom
jwilson_0322_classadapter

Conversation

@swankjesse
Copy link
Collaborator

This borrows from Gson's UnsafeAllocator. I didn't actually
borrow much from Gson's reflective type adapter, but I'll need
to review that in follow up to see if I forgot anything that
Gson covers.

Most interesting design decision here is that fields are
serialized in alphabetical order. Also we're pretty nice
around detecting field collisiosn and failing early.

This borrows from Gson's UnsafeAllocator. I didn't actually
borrow much from Gson's reflective type adapter, but I'll need
to review that in follow up to see if I forgot anything that
Gson covers.

Most interesting design decision here is that fields are
serialized in alphabetical order. Also we're pretty nice
around detecting field collisiosn and failing early.
@swankjesse swankjesse force-pushed the jwilson_0322_classadapter branch from 9be0226 to 08becc1 Compare March 23, 2015 04:13
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super minor nit: we'd normally call this 'ignored'

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i see the others now. interesting convention.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think its probably a dumb convention ‘cause I still need the explanatory comment to prevent a warning.

@JakeWharton
Copy link
Collaborator

CI is broken. Will fix tomorrow.

LGTM

swankjesse added a commit that referenced this pull request Mar 23, 2015
@swankjesse swankjesse merged commit af10c97 into master Mar 23, 2015
@swankjesse swankjesse deleted the jwilson_0322_classadapter branch March 25, 2015 02:04
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.

2 participants