-
-
Notifications
You must be signed in to change notification settings - Fork 348
[WIP] Update to schemars 1.0 #1780
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Danil-Grigorev <[email protected]>
Signed-off-by: Danil-Grigorev <[email protected]>
kube-core/src/schema.rs
Outdated
/// A JSON Schema object. | ||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default, JsonSchema)] | ||
#[serde(rename_all = "camelCase", default)] | ||
pub struct SchemaObject { |
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.
Presumably many of these types here were lifted from schemars 0.8?
Are they necessary for us or is it mostly an api convenience?
Secondly, are they necessary to stay here as pub
? They don't seem used in the examples. EDIT: tried out; you can basically remove pub
from all of them except the original StructuralSchemaRewriter
.
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 removed those pub
keywords. It is needed to preserve logic unchanged, it works well, and edge cases where it fails are related (sourced) to the schemars changes regarding updates to serde(flatten)
handling. I can expand on the issue later if needed.
These structures come from v0.8
tag in schemars
, although I can also remove dangling parts which are not currently used if the size of added definitions matters. Simply having them in place makes implementing overrides easier, then operating on raw serde_json::Value
, and I suspect rewriting current logic to the upstream suggested migration guide would still require implementing similar structures there and then.
Signed-off-by: Danil-Grigorev <[email protected]>
b25d329
to
1c46714
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1780 +/- ##
=======================================
+ Coverage 76.5% 76.5% +0.1%
=======================================
Files 84 84
Lines 7896 7915 +19
=======================================
+ Hits 6038 6054 +16
- Misses 1858 1861 +3
🚀 New features to boost your workflow:
|
Motivation
Manually update
schemars
to 1.0 to support future changes.Solution
Re-implement some of the breaking changes to allow existing
CRD
parsing logic to be preserved, as they rely on structured schema instead ofserde_json::Value
.