-
Notifications
You must be signed in to change notification settings - Fork 80
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
deer
: implement Deserialize
for &[u8]
#2389
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2389 +/- ##
==========================================
+ Coverage 56.76% 60.88% +4.11%
==========================================
Files 336 318 -18
Lines 26012 24265 -1747
Branches 421 421
==========================================
+ Hits 14766 14774 +8
+ Misses 11241 9486 -1755
Partials 5 5
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 23 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
// this type does not really exist in json-schema :/ | ||
// TODO: correct valid schema? | ||
Schema::new("bytes") |
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.
Do you have any ideas for how we could handle this in JSON Schema? And would it be problematic for us to be using &[u8]
for JSON?
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.
&[u8]
is invalid JSON; depending on the library, it is either rejected (JustJson) or strings that are not valid utf8 are passed as bytes (serde-json
).
In the future, I plan to introduce a more robust system for schemas (project blueprint™️) that would solve this ambiguity by being agnostic over the output schema and having JSON-Schema be one of the output schemas.
For JSON-schema, specifically bytes data is typically encoded via contentEncoding
(https://json-schema.org/understanding-json-schema/reference/non_json_data.html) so you would specify as encoding base64 and then deserialize into bytes.
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.
I think we can merge as-is still regardless of question above, but would like thoughts on it still :)
🌟 What is the purpose of this PR?
Implements deserialize for
&[u8]
to allow for borrowed byte slices, taken from #1875🔗 Related links
deer
type implementation #1700deer
implementDeserialize
for built-in types (Part 2) #1875🔍 Has this modified a publishable library?
This PR: