Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove usage of _neitherNull on findProperty #1913

Closed
wants to merge 0 commits into from
Closed

Remove usage of _neitherNull on findProperty #1913

wants to merge 0 commits into from

Conversation

deblockt
Copy link
Contributor

@deblockt deblockt commented Jan 29, 2018

Remove usage of _neitherNull on findProperty to get custom property if possible.

close #1912

@deblockt deblockt changed the title Fix #1912 Remove usage of _neitherNull on findProperty Jan 29, 2018
@cowtowncoder
Copy link
Member

Thank you for contributing this. Looks legit.

But would it be possible to get a unit test, to show the effect, and guard against regression? (... by refactoring such as the one that broke handling).

@cowtowncoder
Copy link
Member

One other minor thing: I'm happy to merge this, but one thing we need before the first contribution to Jackson projects is CLA: (unless we already got one -- apologies if I missed earlier one, tried to check)

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

Usual way is to print, fill, sign, scan & email toinfo at fasterxml dot com.
There is also alternative Corporate CLA if you prefer, most contributions are under individual one.

@deblockt
Copy link
Contributor Author

deblockt commented Jan 30, 2018

@cowtowncoder I will add unit test.

I have see that master is under 3.0.0-SNAPSHOT version.
If I want this bug to be fixed on the next version of Jackson, which branch should I merge on? Is it 2.9 branch?

@cowtowncoder
Copy link
Member

I'd guess this was done for 2.9, so branch would be 2.9, which is easy to merge to master then.
I can handle backporting if need be.

So I just need the CLA, whenever you get a chance (it's just one time thing, not needed for further contributions. But needed since big corporations want to ensure all code contributions are done properly wrt licensing etc).

Thank you again!

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.

BeanDeserializerModifier.updateBuilder() not work to set custom deserializer on a property (since 2.9.0)
2 participants