search

FasterXML / jackson-dataformats-binary

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

Fix IonValueDeserializer's getNullValue when bean is missing a property #320

Closed atokuzAmzn closed 2 years ago

atokuzAmzn commented 2 years ago

This PR aims to resolve the issue https://github.com/FasterXML/jackson-dataformats-binary/issues/317

Changes:

atokuzAmzn commented 2 years ago

@mcliedtke Can you review this?

mcliedtke commented 2 years ago

Can you also comment on what branches you believe this change to be added to. Are you targeting for updating 2.13?

atokuzAmzn commented 2 years ago

@mcliedtke good catch on missing check. I have added the extra assertion. Yes we are targeting to update 2.13

mcliedtke commented 2 years ago

LGTM, I think these changes can be merged. Given the nature of the change, I think merging to 2.13 is pretty safe. @cowtowncoder do you want to handle the merge itself?

atokuzAmzn commented 2 years ago

@cowtowncoder or @mcliedtke Can we merge this into 2.13?

atokuzAmzn commented 2 years ago

Following up with this merge request @cowtowncoder @mcliedtke

cowtowncoder commented 2 years ago

Ok, I am back from my vacation. @atokuzAmzn If you want this merged against 2.13, please rebase or recreate. While cherry-picking sometimes works, the usual practice is to start with the oldest branch & merge forward.

atokuzAmzn commented 2 years ago

This is my first time working with github. I changed the base of this PR but I am not sure if this is what you wanted @cowtowncoder. If needed I can recreate a new PR based on 2.13

cowtowncoder commented 2 years ago

@atokuzAmzn Alas, no: this would merge back all changes from 2.14 -- and not just ones you are making -- so that wouldn't work. I am not 100% sure what'd be the easiest way to change the base for existing PR. Maybe others can suggest something?

But it may be easier to simply create a new PR, if that works?

atokuzAmzn commented 2 years ago

I created a new PR based on 2.13 here : https://github.com/FasterXML/jackson-dataformats-binary/pull/322