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

add configuration for xsi:nil="true" conversion to null #467

Merged

Conversation

meiskalt7
Copy link
Contributor

@meiskalt7 meiskalt7 commented Apr 9, 2019

Hello :-)

I add configuration for xsi:nil="true" conversion to null. In JSON spec null should be represented like
{ key: null }
link: https://stackoverflow.com/questions/21120999/representing-null-in-json

Please check it and merge, cause it is necessary for many dependent projects.

What problem does this code solve?
Representation of null in accordance with JSON spec

Risks
Low

Changes to the API?
No, only addition new constructor for XMLParserConfiguration

Will this require a new release?
Yes.

Should the documentation be updated?
Yes. Javadoc already updated.

Does it break the unit tests?
No

Was any code refactored in this commit?
Yes, changes were made to the number parsing methods for XML and added new configuration
(no changes in public methods)

Backward compatibility
Yes

Review status
APPROVED

@meiskalt7 meiskalt7 force-pushed the addConfigurationForXsiNilConversionToNull branch from a563fae to 1773717 Compare April 9, 2019 12:38
@meiskalt7 meiskalt7 force-pushed the addConfigurationForXsiNilConversionToNull branch from 1773717 to d5b2785 Compare April 9, 2019 12:49
@stleary
Copy link
Owner

stleary commented Apr 13, 2019

HI @meiskalt7. Thanks for submitting this PR, it is appreciated, and sorry for not responding sooner. Looks like no one on the team has a strong opinion regarding this change. Your change does not break unit tests, so that is good, but you will need to add additional unit tests if you want to proceed. Please refer to the FAQ.

@meiskalt7
Copy link
Contributor Author

Thanks for your answer. I will add tests for this changes ASAP.

@meiskalt7
Copy link
Contributor Author

meiskalt7 commented Apr 18, 2019

I added test for this pull request
link: stleary/JSON-Java-unit-test#93

@stleary
Copy link
Owner

stleary commented Apr 20, 2019

Approved. Starting 3 day comment window.

@stleary stleary merged commit 00e0e6c into stleary:master Apr 22, 2019
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.

None yet

2 participants