-
-
Notifications
You must be signed in to change notification settings - Fork 370
Rust 2024 let-chains to simplify wait Conditions #1792
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
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1792 +/- ##
=======================================
- Coverage 74.7% 74.7% -0.0%
=======================================
Files 84 84
Lines 7951 7950 -1
=======================================
- Hits 5939 5937 -2
- Misses 2012 2013 +1
🚀 New features to boost your workflow:
|
started out as a PR to try out let-chains, but found that it solves the ergonomic problem with the wait API that I tried to solve earlier in #1498 Signed-off-by: clux <[email protected]>
d9571ab to
f42dfcc
Compare
Signed-off-by: clux <[email protected]>
Signed-off-by: clux <[email protected]>
Signed-off-by: clux <[email protected]>
Signed-off-by: clux <[email protected]>
Signed-off-by: clux <[email protected]>
kube-core/src/params.rs
Outdated
| match &self.version_match { | ||
| None => {} | ||
| Some(VersionMatch::NotOlderThan) => { | ||
| qp.append_pair("resourceVersionMatch", "NotOlderThan"); | ||
| } | ||
| Some(VersionMatch::Exact) => { | ||
| qp.append_pair("resourceVersionMatch", "Exact"); | ||
| } |
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.
could technically be
| match &self.version_match { | |
| None => {} | |
| Some(VersionMatch::NotOlderThan) => { | |
| qp.append_pair("resourceVersionMatch", "NotOlderThan"); | |
| } | |
| Some(VersionMatch::Exact) => { | |
| qp.append_pair("resourceVersionMatch", "Exact"); | |
| } | |
| if let Some(vm) = &self.version_match { | |
| qp.append_pair("resourceVersionMatch", &format!("{:?}", vm)); | |
| } |
but not sure it's better. feels it drops some readability, and don't like relying on debug print.
Danil-Grigorev
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.
One suggestion, otherwise looks good.
Signed-off-by: clux <[email protected]>
Bumps MSRV to 1.88.0 and converts existing let-chainable code to use let-chains. Rust 2024 support was added in separately in #1856, and we can use it properly now since 1.88 is like 3 versions behind.
Was originally playing around with it, but actually found that it solves most of the the ergonomic problem with the wait API that I tried to solve earlier in #1498 - it's not quite as elegant but it also doesn't come with any of the problems and is non-breaking.
But if the general setup (outlined in the first commit) is acceptable, I'll rebase at 1.90.Waiting for #1856 to merge.Now ready.