search

FasterXML / jackson-dataformats-binary

Uber-project for standard Jackson binary format backends: avro, cbor, ion, protobuf, smile
Apache License 2.0
314 stars 136 forks source link

Fix serialization of union types at root level (fixes #168) #176

Closed marcospassos closed 5 years ago

marcospassos commented 5 years ago

Rebased for inclusion in 2.10. Fixes #168.

marcospassos commented 5 years ago

@cowtowncoder could you include this fix in 2.10? We've been using it since then and no issues found so far.

cowtowncoder commented 5 years ago

@marcospassos Apologies for slow follow-up: this (and couple of other) Avro issues are on my biggish todo list (https://github.com/FasterXML/jackson-future-ideas/wiki/Jackson-Work-in-Progress) but have been struggling with a few other issues.

But, I'll have a look at this (wrt #168) and #173 (for #175).

One last thing: have I yet asked for (and received) CLA? If not, it's at:

https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf

(or, if you prefer, there's corporate-CLA too although most contributions are with individual) and the common way is to print it, fill, sign & scan, email to info at fasterxml dot com. When I get it I can merge all contributions for any projects, it's just first-time thing we need.

Looking forward to merging these PRs!

cowtowncoder commented 5 years ago

I didn't see anything odd so I think it's good to go.

One suggestion: if there are old methods left in for backwards compatibility, but that may eventually be removed (in 3.0), it makes sense to add @Deprecated (I am not sure if that is the case for factory methods that did not take currValue). I can easily drop those from 3.0 after merging.

marcospassos commented 5 years ago

@cowtowncoder I just sent you the CLA.

One suggestion: if there are old methods left in for backwards compatibility, but that may eventually be removed (in 3.0), it makes sense to add @Deprecated (I am not sure if that is the case for factory methods that did not take currValue). I can easily drop those from 3.0 after merging.

I don't think it's the case here.