Conversation
Not requiring this probably isn't very useful (as there are most likely not many use cases where `Project` needs to contain any data; yet alone data that cannot be `Send`). With `Project: Send`, the bootstrapper is `Send` as well which is useful when you want to run the server in tokio::spawn (for instance, as part of an already running async program) instead of tokio::block_on.
|
See m4tx/tundra#173 on why this is useful. |
There was a problem hiding this comment.
Pull request overview
This PR makes Project require Send so that Bootstrapper (and related async boot/run flows) can be Send, enabling use cases like running the server inside tokio::spawn rather than requiring tokio::block_on.
Changes:
- Add
Sendas a supertrait bound onProject. - Remove now-unnecessary
clippy::future_not_sendsuppressions inBootstrapperboot/run paths and in tests/utilities. - Keep/adjust remaining clippy attributes where
unused_asyncis still intentionally expected.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| cot/src/project.rs | Makes Project: Send and removes future_not_send expectations from bootstrap/run APIs impacted by Project’s sendability. |
| cot/src/test.rs | Removes future_not_send expectation from Client::new now that Project is Send. |
| cot/src/middleware/live_reload.rs | Removes future_not_send expectation from a helper test that uses Bootstrapper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cot/src/project.rs
Outdated
| /// } | ||
| /// ``` | ||
| pub trait Project { | ||
| pub trait Project: Send { |
There was a problem hiding this comment.
Project is a public trait and adding : Send is a breaking API change for downstream implementors that were using Rc/RefCell or other !Send state. This repo’s CHANGELOG follows SemVer/Keep-a-Changelog and typically labels breaking changes; please add an entry under [Unreleased] marking this as breaking (and ensure the release/versioning process accounts for it).
| pub trait Project: Send { | |
| pub trait Project { |
|
Bencher Report| Branch | project-send |
| 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,761.20 µs(-2.67%)Baseline: 5,919.52 µs | 7,094.56 µs (81.21%) |
| json_api/json_api | 📈 view plot 🚷 view threshold | 1,001.60 µs(-1.30%)Baseline: 1,014.83 µs | 1,161.51 µs (86.23%) |
| nested_routers/nested_routers | 📈 view plot 🚷 view threshold | 940.47 µs(+0.53%)Baseline: 935.49 µs | 1,064.37 µs (88.36%) |
| single_root_route/single_root_route | 📈 view plot 🚷 view threshold | 907.62 µs(+1.22%)Baseline: 896.70 µs | 1,022.77 µs (88.74%) |
| single_root_route_burst/single_root_route_burst | 📈 view plot 🚷 view threshold | 16,611.00 µs(-5.33%)Baseline: 17,546.21 µs | 20,828.60 µs (79.75%) |
m4tx
changed the title
Project require SendProject require Send
Codecov Report❌ Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
1 participant
Not requiring this probably isn't very useful (as there are most likely not many use cases where
Projectneeds to contain any data; yet alone data that cannot beSend). WithProject: Send, the bootstrapper isSendas well which is useful when you want to run the server in tokio::spawn (for instance, as part of an already running async program) instead of tokio::block_on.EDIT: After a consideration, changed the
Bootstrapperto requireProject + Sendinstead of adding the requirement to theProjecttrait itself, which could be useful if we found a good use case for using!Sendproject in the future and wanted to use it without breaking APIs.