-
Notifications
You must be signed in to change notification settings - Fork 758
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
Improve JSON to Record Conversion #43482
Improve JSON to Record Conversion #43482
Conversation
|
||
TypeDescriptorNode nonAnyDataNode = null; | ||
TypeDescriptorNode nonJSONDataNode = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we follow the standard camelCase?
TypeDescriptorNode nonJSONDataNode = null; | |
TypeDescriptorNode nonJsonDataNode = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I will fix that soon
There is a checkstyle failure as well, shall we check that as well? |
I was a bit unsure about the open/closeness of the records since in the json-mapper the isClosed is also applied into inner records but in the language server json-to-record-converter this is only applied on the outer most level and all inner records were open. I now adjusted the language server module to be the same as json-mapper since I think this approach makes more sense. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #43482 +/- ##
============================================
- Coverage 77.55% 77.55% -0.01%
+ Complexity 58723 58722 -1
============================================
Files 3447 3447
Lines 219656 219658 +2
Branches 28917 28915 -2
============================================
+ Hits 170364 170365 +1
- Misses 39891 39892 +1
Partials 9401 9401 ☔ View full report in Codecov by Sentry. |
@Shadow-Devil Shall we fix the code conflict please? |
# Conflicts: # misc/ls-extensions/modules/json-to-record-converter/src/main/java/io/ballerina/converters/JsonToRecordConverter.java
@gimantha code conflicts are fixed and pipeline was successful |
Purpose
Fixes #42610
Approach
Samples
Remarks
Check List