-
-
Notifications
You must be signed in to change notification settings - Fork 411
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
[Merged by Bors] - Added conversions from and to serde_json's Value type #1851
Conversation
Test262 conformance changesVM implementation
|
Codecov Report
@@ Coverage Diff @@
## main #1851 +/- ##
==========================================
+ Coverage 56.09% 56.29% +0.19%
==========================================
Files 200 201 +1
Lines 17901 17936 +35
==========================================
+ Hits 10042 10097 +55
+ Misses 7859 7839 -20
Continue to review full report at Codecov.
|
Benchmark for f680474Click to view benchmark
|
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.
Great! This should be pretty useful for users of serde_json
:D
Also, I think some module tests would be good to ensure conversions work OK |
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.
Nothing to add besides @jedel1043's notes
I wrote the fixes and added a pretty deep test. Hopefully this should solve most situations. |
Benchmark for 6081edfClick to view benchmark
|
Benchmark for 12bac5eClick to view benchmark
|
Benchmark for 8eff2bcClick to view benchmark
|
Benchmark for 55eb70fClick to view benchmark
|
Benchmark for 6be7440Click to view benchmark
|
Benchmark for d0593f4Click to view benchmark
|
bors r+ |
🕐 Waiting for PR status (Github check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set. |
This Pull Request closes #1693. It changes the following: - It adds a fallible conversion from `serde_json::Value` to `JsValue`, which requires a context. - It adds a fallible conversion from `JsValue` to `serde_json::Value`, which requires a context. - Added examples to the documentation of both methods. - Removed some duplicate and non-needed code that I found while doing this. Co-authored-by: RageKnify <RageKnify@gmail.com>
Pull request successfully merged into main. Build succeeded: |
Benchmark for ab9d80dClick to view benchmark
|
This Pull Request closes #1693.
It changes the following:
serde_json::Value
toJsValue
, which requires a context.JsValue
toserde_json::Value
, which requires a context.