-
-
Notifications
You must be signed in to change notification settings - Fork 35
refactor!: use trait_upcasting, bump MSRV to 1.86
#412
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
|
| Branch | trait_upcasting |
| Testbed | github-ubuntu-latest |
Click to view all benchmark results
| Benchmark | Latency | Benchmark Result microseconds (µs) (Result Δ%) | Upper Boundary microseconds (µs) (Limit %) |
|---|---|---|---|
| empty_router/empty_router | 📈 view plot 🚷 view threshold | 5,000.20 µs(-17.13%)Baseline: 6,034.02 µs | 6,783.39 µs (73.71%) |
| json_api/json_api | 📈 view plot 🚷 view threshold | 846.16 µs(-17.15%)Baseline: 1,021.31 µs | 1,139.01 µs (74.29%) |
| nested_routers/nested_routers | 📈 view plot 🚷 view threshold | 795.50 µs(-16.15%)Baseline: 948.77 µs | 1,059.73 µs (75.07%) |
| single_root_route/single_root_route | 📈 view plot 🚷 view threshold | 754.67 µs(-16.66%)Baseline: 905.51 µs | 1,008.75 µs (74.81%) |
| single_root_route_burst/single_root_route_burst | 📈 view plot 🚷 view threshold | 13,746.00 µs(-20.98%)Baseline: 17,395.41 µs | 21,436.10 µs (64.13%) |
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.
Pull Request Overview
This PR updates the minimum supported Rust version (MSRV) from 1.85 to 1.86 to leverage the stabilized trait upcasting feature, eliminating the need for workaround methods that were previously required.
- Updates MSRV to Rust 1.86 in Cargo.toml and CI configuration
- Removes
as_box_request_handler()workaround method from OpenAPI traits - Removes
as_any()workaround method from AdminModel trait and its implementations
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Cargo.toml | Updates rust-version to 1.86 |
| .github/workflows/rust.yml | Updates MSRV version in test matrix to 1.86 |
| cot/src/openapi.rs | Removes as_box_request_handler() method from BoxApiEndpointRequestHandler trait |
| cot/src/router.rs | Uses direct trait upcasting instead of as_box_request_handler() call |
| cot/src/admin.rs | Removes as_any() method from AdminModel trait and updates downcasting logic |
| cot-macros/src/admin.rs | Removes as_any() implementation from AdminModel derive macro |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cot/src/admin.rs
Outdated
| .as_any() | ||
| .downcast_ref::<T>() | ||
| .expect("Invalid object type"); | ||
| let object_any: &dyn Any = &object; |
Copilot
AI
Nov 5, 2025
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.
The downcasting logic is incorrect. object is a Box<dyn AdminModel>, and taking a reference to it (&object) creates a &Box<dyn AdminModel>, not &dyn AdminModel. The code should dereference the box first: let object_any: &dyn Any = &*object; to perform trait upcasting from &dyn AdminModel to &dyn Any before downcasting to &T.
| let object_any: &dyn Any = &object; | |
| let object_any: &dyn Any = &*object; |
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.
Pull Request Overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests.
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
No description provided.