Conversation
There was a problem hiding this comment.
Pull request overview
This PR migrates the Rust crate to the Rust 2024 edition and makes small style refactors in a couple of async/PyO3-facing code paths.
Changes:
- Update
rust/Cargo.tomlto useedition = "2024". - Refactor a small async return path in
Platform::get_app_channelto usematch. - Refactor Python TOC cache extraction logic in
Crazyflie::connect_from_uriinto nestedmatchexpressions.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
rust/Cargo.toml |
Switch crate edition to Rust 2024. |
rust/src/subsystems/platform.rs |
Minor refactor of get_app_channel optional return handling. |
rust/src/crazyflie.rs |
Refactor TOC cache extraction branching logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| name = "cflib-rust" | ||
| version = "0.1.0" | ||
| edition = "2021" | ||
| edition = "2024" |
There was a problem hiding this comment.
Switching the crate to edition 2024 effectively raises the minimum supported Rust toolchain (older compilers will fail to parse/build). To make this explicit and avoid confusing build failures in downstream environments, add an explicit MSRV (e.g., rust-version) and/or a repo-level toolchain pin (rust-toolchain.toml) that matches the intended minimum.
| edition = "2024" | |
| edition = "2024" | |
| rust-version = "1.85" |
| let cache = match toc_cache { | ||
| Some(cache_obj) => match cache_obj.extract::<NoTocCache>() { | ||
| Ok(no_cache) => AnyCacheWrapper::NoCache(no_cache), | ||
| _ => match cache_obj.extract::<InMemoryTocCache>() { | ||
| Ok(mem_cache) => AnyCacheWrapper::InMemory(mem_cache), | ||
| _ => match cache_obj.extract::<FileTocCache>() { | ||
| Ok(file_cache) => AnyCacheWrapper::File(file_cache), | ||
| _ => return Err(PyRuntimeError::new_err( | ||
| "toc_cache must be NoTocCache, InMemoryTocCache, or FileTocCache" | ||
| )), | ||
| }, | ||
| }, | ||
| }, | ||
| None => AnyCacheWrapper::NoCache(NoTocCache), |
There was a problem hiding this comment.
The new nested match chain for toc_cache extraction increases indentation and is harder to read/extend than the previous sequential checks. Consider refactoring back to a flat sequence (e.g., if let Ok(...) / else if ...) or using combinators/helper function to try each extract in order, to keep control flow simpler.
| let cache = match toc_cache { | |
| Some(cache_obj) => match cache_obj.extract::<NoTocCache>() { | |
| Ok(no_cache) => AnyCacheWrapper::NoCache(no_cache), | |
| _ => match cache_obj.extract::<InMemoryTocCache>() { | |
| Ok(mem_cache) => AnyCacheWrapper::InMemory(mem_cache), | |
| _ => match cache_obj.extract::<FileTocCache>() { | |
| Ok(file_cache) => AnyCacheWrapper::File(file_cache), | |
| _ => return Err(PyRuntimeError::new_err( | |
| "toc_cache must be NoTocCache, InMemoryTocCache, or FileTocCache" | |
| )), | |
| }, | |
| }, | |
| }, | |
| None => AnyCacheWrapper::NoCache(NoTocCache), | |
| let cache = if let Some(cache_obj) = toc_cache { | |
| if let Ok(no_cache) = cache_obj.extract::<NoTocCache>() { | |
| AnyCacheWrapper::NoCache(no_cache) | |
| } else if let Ok(mem_cache) = cache_obj.extract::<InMemoryTocCache>() { | |
| AnyCacheWrapper::InMemory(mem_cache) | |
| } else if let Ok(file_cache) = cache_obj.extract::<FileTocCache>() { | |
| AnyCacheWrapper::File(file_cache) | |
| } else { | |
| return Err(PyRuntimeError::new_err( | |
| "toc_cache must be NoTocCache, InMemoryTocCache, or FileTocCache" | |
| )); | |
| } | |
| } else { | |
| AnyCacheWrapper::NoCache(NoTocCache) |
There was a problem hiding this comment.
cargo fix --edition did this for me,
No description provided.