search

rharter / auto-value-gson

AutoValue Extension to add Gson De/Serializer support
Apache License 2.0
607 stars 103 forks source link

Support optional #148

Closed acdebaca closed 5 years ago

acdebaca commented 7 years ago

This is a first-cut implementation for issue #123, adding support for optional types. This implementation supports the following optional types:

com.google.common.base.Optional java.util.Optional java.util.OptionalDouble java.util.OptionalInt java.util.OptionalLong

Regression tests have been added to cover various combinations of features.

rharter commented 7 years ago

This looks really good, overall, just a couple of discussion points.

gabrielittner commented 7 years ago

Might be worth it to bump https://github.com/google/auto/issues/359 so that AutoValue and this can share the same logic around this.

acdebaca commented 6 years ago

Hi @rharter - I know it's been a few months, but I'd still love to get this code up-streamed if possible. Please review and let me know how you'd like for me to proceed. Thanks!

rharter commented 6 years ago

Hi @acdebaca, sorry for letting this languish. I think you've cleared things up for me and I'm happy to merge this once the conflicts are resolved. Thanks!

acdebaca commented 6 years ago

@rharter Great! I'll update this discussion when the changes are ready.

epool commented 6 years ago

@acdebaca firstly thank you so much for this helpful PR, but, do you have an ETA for the conflicts resolution? Thanks again!

acdebaca commented 6 years ago

@epool I don't have an ETA on this, but I'll see if I can get them fixed in one sitting today or tomorrow.

kaibolay commented 6 years ago

@acdebaca I'm looking for this feature. I hope you find some time to clean up the merge conflicts.

If you don't have time, would you mind if someone else (e.g. me) tried to resolve the conflicts and get this merged?

epool commented 6 years ago

@kaibolay I ended up using kotlin data classes instead of autovalue.

rharter commented 6 years ago

@kaibolay feel free to check out this branch and fix the merge conflicts and resubmit. Would be appreciated!

ZacSweers commented 5 years ago

I think we should make a call for now to just not support these. Modeling absence in general Java code is one thing, but modeling it in JSON is very different, and I'd rather opt for no solution rather than a limited one and encourage better support for normalization from AutoValue via #132.