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.

Sign up for GitHub

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

Jump to bottom

Support unwrapping arrays for JsonNode decoder #747

Merged
merged 4 commits into from
Feb 1, 2024
Merged

Support unwrapping arrays for JsonNode decoder #747

merged 4 commits into from
Feb 1, 2024

Conversation

dstepanov
Copy link
Contributor

Fixes #699

  • Added new methods to read input directly into the JsonNode

@yawkat
Copy link
Member

yawkat commented Jan 31, 2024

Unwrapping LGTM.

I don't understand the point of the new tree methods. They convert bytes->tree, but you can already use readValue(bytes, JsonNode.class) for that. this seems to have an additional conversion step to an intermediate value, but i dont think it is necessary?

@dstepanov
Copy link
Contributor Author

dstepanov commented Jan 31, 2024 •
edited
Loading

@yawkat I didn't realize it can be used that way, removed the new methods

@yawkat
Copy link
Member

yawkat commented Jan 31, 2024

Maybe it makes sense to have aliases for it, but the impl can delegate to readValue(x, JsonNode) instead of going through another type.

@dstepanov dstepanov requested a review from yawkat January 31, 2024 16:40
@dstepanov @yawkat
Update serde-oracle-jdbc-json/src/main/java/io/micronaut/serde/oracle…
0fa0fe9 
…/jdbc/json/AbstractOracleJdbcJsonObjectMapper.java

Co-authored-by: Jonas Konrad <jonas.konrad@oracle.com>
@sonarqubecloud SonarQubeCloud
Copy link

sonarqubecloud bot commented Feb 1, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
71.5% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@dstepanov dstepanov merged commit cf00261 into master Feb 1, 2024
9 checks passed
@dstepanov dstepanov deleted the readt branch February 1, 2024 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yawkat yawkat yawkat approved these changes

@graemerocher graemerocher graemerocher approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ConfigurationProperties regression for nested config using jsonp serde
3 participants
@dstepanov @yawkat @graemerocher