Skip to content
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

Swap validator for garde #1212

Merged
merged 8 commits into from
Apr 29, 2023
Merged

Conversation

mateiidavid
Copy link
Contributor

Motivation

The validator crate used in the derive docs and in the crd example is no longer actively maintained. It has been superseded by garde which offers similar functionality.

Fixes #1197 (check issue for additional context)

Solution

This change replaces validator with garde in dev dependencies (and in the crd_api example dependency). Although garde has the same derive macro, its attribute differs from validator, and it additionally requires fields to be explictly skipped from validation, and does not offer a built-in way to validate required fields.

Changes have been made to the kube_derive crate's docs to both replace existing mentions of validator, and inform on the caveats mentioned above.

The validator crate used in the derive docs and in the crd example is no
longer actively maintained. It has been superseded by garde which offers
similar functionality.

This change replaces validator with garde in dev dependencies (and in
the crd_api example dependency). Although garde has the same derive
macro, its attribute differs from validator, and it additionally
requires fields to be explictly skipped from validation, and does not
offer a built-in way to validate required fields.

Changes have been made to the kube_derive crate's docs to both replace
existing mentions of validator, and inform on the caveats mentioned
above.

Signed-off-by: Matei David <matei.david.35@gmail.com>
@codecov
Copy link

codecov bot commented Apr 28, 2023

Codecov Report

Merging #1212 (b7275a4) into main (e01187e) will decrease coverage by 0.04%.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1212      +/-   ##
==========================================
- Coverage   73.49%   73.45%   -0.04%     
==========================================
  Files          68       68              
  Lines        5349     5341       -8     
==========================================
- Hits         3931     3923       -8     
  Misses       1418     1418              
Impacted Files Coverage Δ
kube-derive/src/lib.rs 0.00% <ø> (ø)
kube/src/lib.rs 89.04% <ø> (ø)

... and 8 files with indirect coverage changes

Signed-off-by: Matei David <matei.david.35@gmail.com>
Copy link
Member

@clux clux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking the time to dig into this. I think that given how clearly this ends up separating client-side and server-side validation, i think we should take garde out of our dev-dependencies and leave it as an example trick, but the docs are good. Left one bigger reword.

kube/src/lib.rs Outdated Show resolved Hide resolved
kube/Cargo.toml Outdated Show resolved Hide resolved
kube-derive/src/lib.rs Outdated Show resolved Hide resolved
@clux clux added this to the 0.83.0 milestone Apr 29, 2023
examples/crd_api.rs Outdated Show resolved Hide resolved
mateiidavid and others added 6 commits April 29, 2023 11:49
Signed-off-by: Matei David <matei.david.35@gmail.com>
Signed-off-by: Matei David <matei.david.35@gmail.com>
Co-authored-by: Eirik A <sszynrae@gmail.com>
Signed-off-by: Matei David <matei.david.35@gmail.com>
Signed-off-by: Matei David <matei.david.35@gmail.com>
Signed-off-by: Matei David <matei.david.35@gmail.com>
@mateiidavid
Copy link
Contributor Author

@clux thanks for the review! I made a few changes:

  • removed garde from dev deps
  • replaced #[validate] with #[schemars] in both comments and code
  • re-worked the docs a bit

Lmk how it looks, happy to finesse this more :)

Copy link
Member

@clux clux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great now. Thank you!

@clux clux added the changelog-fix changelog fix category for prs label Apr 29, 2023
@clux clux merged commit 3931c24 into kube-rs:main Apr 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog-fix changelog fix category for prs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Swap validator for garde
2 participants