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

SmileParser getValueAsString() issue with JsonToken.FIELD_NAME #540

Merged

Conversation

johhud1
Copy link
Contributor

@johhud1 johhud1 commented Dec 30, 2024

Test updates to reproduce the issue and naive fix attempt for #541

@cowtowncoder cowtowncoder added smile cla-needed PR looks good (although may also require code review), but CLA needed from submitter labels Dec 31, 2024
@cowtowncoder
Copy link
Member

Ok I'll go over the PR; I think it makes sense overall.

But one process thing we need before I can merge the fix (assuming review goes fine); unless already done, we'd need a CLA. It's here:

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

and only needs to be sent one before the first contribution is merged (good for all future PRs).
The usual way is to print it, fill & sign, scan/photo, email to cla at fasterxml dot com.
Once I get it can proceed with merging (and probably in this case also backporting in 2.18 at least, maybe 2.17)

@johhud1
Copy link
Contributor Author

johhud1 commented Jan 3, 2025

Just emailed my signed CLA

@cowtowncoder cowtowncoder removed the cla-needed PR looks good (although may also require code review), but CLA needed from submitter label Jan 4, 2025
@cowtowncoder
Copy link
Member

CLA received.

Copy link
Member

@cowtowncoder cowtowncoder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good; will extend a bit to handle other formats too.

@cowtowncoder
Copy link
Member

Ok: added tests to ensure all 5 format backends work (as well as test for JsonParser.getValueAsString(String defaultValue)) -- and matching fixes (some worked, some didn't).

Thank you for reporting this @johhud1 and submitting a good fix for Smile case.

@cowtowncoder cowtowncoder merged commit e5f1a17 into FasterXML:2.19 Jan 4, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants