-
Notifications
You must be signed in to change notification settings - Fork 118
Bump schemars dependency to 1.2 #4126
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
Conversation
2e8ee1d to
3b7a754
Compare
623ee11 to
593ed26
Compare
slinkydeveloper
left a comment
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.
Small comment, and then LGTM
crates/types/src/net/address.rs
Outdated
| fn json_schema(_generator: &mut schemars::SchemaGenerator) -> schemars::Schema { | ||
| schemars::json_schema!({ | ||
| "type": "string", | ||
| "format": "ip:port", |
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.
A note about formats (here and in other places).
format is a json schema keyword which accepts either "well defined formats" or "custom formats". Problem is, some codegens won't like custom formats, and might behave erratically with those.
So please remove all the usages of "custom formats", and just keep the "formats" when they're part of this well-defined list: https://json-schema.org/draft/2020-12/draft-bhutton-json-schema-validation-00#rfc.section.7.3
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.
Thanks for the pointer. Ok, then I'll remove the custom formats.
This fixes restatedev#3766.
While custom formats are supported by JSON schema, it makes some code generator tools to trip over them. Hence, we are removing custom formats from the code base.
2b1ca74 to
7a95273
Compare
This PR is based on #4125.
This fixes #3766.