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

Duration cleanup and doctests #47

Merged
merged 7 commits into from
Aug 20, 2024

Conversation

kflynn
Copy link
Contributor

@kflynn kflynn commented Aug 16, 2024

This is best reviewed commit by commit.

  • Pull the unneeded "use delegate::delegate" that didn't quite get removed before my previous PR merged
  • Move Duration to gateway-api/src/duration.rs
  • Switch to meaningful variable names in the TryFrom implementations
  • Oh, yeah, run cargo fmt, that's a thing 🤦‍♂️
  • Clean up some test comments and reorder test_gep2257_from_valid_duration cases to match test_gep2257_from_str
  • Finally, do a monster doc hopefully-improvement and add doctests

Copy link
Member

@shaneutt shaneutt left a comment

Choose a reason for hiding this comment

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

Looks good, thank you!

One thing I noticed: we didn't actually enable your tests in CI, but we can't just yet because there's a problem with the generated code when it comes to doctests that I need to take some time and resolve (#49). That doesn't need to hold up this PR though, tests are passing when run manually and I'll get that sorted out shortly.

gateway-api/src/duration.rs Show resolved Hide resolved
@shaneutt shaneutt enabled auto-merge (rebase) August 20, 2024 14:01
@shaneutt shaneutt merged commit d3986f1 into kube-rs:main Aug 20, 2024
2 checks passed
@kflynn kflynn deleted the flynn/dev/update-duration branch August 20, 2024 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants