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

Fix #3503 - Implement Integer to Float coercion config #3509

Merged

Conversation

Tomasito665
Copy link
Contributor

Description

This pull request proposes to update the float, Float, and BigDecimal deserializing logic to take into account the coercion config for integer JSON inputs. Currently, this configuration is being ignored.

Issue

#3503

@cowtowncoder
Copy link
Member

Looks good! One thing I will need before merging in for 2.14 is the CLA from

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

which is usually easiest to do by printing, filling & signing, scan/photo, email to info at fasterxml dot com.
Once this is done (and it is only needed for the first contribution; one works for all contributions), I can proceed with merging.

Looking forward to getting this improvement in, I like it!

@pjfanning
Copy link
Member

@Tomasito665 the code works in Java 11 but not in Java 8 - can you fix it so that the code works in Java 8 too?

@cowtowncoder
Copy link
Member

It is possible that this is due to an issue with Github actions -- to resolve, merge or rebase from 2.14 (there has been a fix) -- and not PR itself.

@pjfanning
Copy link
Member

@Tomasito665 do you intend to continue with this PR or have you abandoned it?

@cowtowncoder
Copy link
Member

I definitely hope we could get this merged, for what that is worth!

@Tomasito665
Copy link
Contributor Author

Heyo! Sorry for the radio silence. Thanks for your comments. Had set this pull request on pause for a while while focusing on work/study. I will rebase this tomorrow on top of 2.14 and see if it works!

@cowtowncoder
Copy link
Member

Thank you @Tomasito665! Also, if and when things work, CLA would be good to get so I can merge PR.

@Tomasito665
Copy link
Contributor Author

Tomasito665 commented Jun 30, 2022

@cowtowncoder I have rebased this branch on top of 2.14. Could you re-trigger CI?

With regards to the CLI CLA, I sent it in on June 4th to info@fasterxml.com from jordi665@hotmail.com.

Let me know if you have found it or, if not, I will resend it.

@cowtowncoder
Copy link
Member

@Tomasito665 doh! CLA had gone into Gmail's spam folder. I should check it now and then. But yes, I got it now!

@cowtowncoder cowtowncoder added this to the 2.14.0 milestone Jul 1, 2022
@cowtowncoder cowtowncoder merged commit 86355cf into FasterXML:2.14 Jul 1, 2022
@cowtowncoder
Copy link
Member

Thank you @Tomasito665 for this contribution: it looks great, I think you got it right, impressive! This will be in 2.14.0 once released, I hope to get the first release candidate out in July.

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.

3 participants