Remove async_trait dependency, MSRV CI #37
Conversation
.github/workflows/test.yml
Outdated
| - name: Read crate metadata | ||
| id: metadata | ||
| run: echo "rust-version=$(sed -ne 's/rust-version *= *\"\(.*\)\"/\1/p' Cargo.toml)" >> $GITHUB_OUTPUT | ||
| - name: Install Rust | ||
| uses: dtolnay/rust-toolchain@stable | ||
| with: | ||
| toolchain: ${{ steps.metadata.outputs.rust-version }} | ||
| components: clippy,rustfmt |
There was a problem hiding this comment.
More of a curiosity than anything, but why not use cargo hack? I found it to be a simpler way to enforce MSRV checks in Rust CI, but that's just a personal preference.
https://github.com/stadiamaps/ferrostar/blob/6ee817cd3dda844715d305897db1b9c04a7b5127/.github/workflows/rust.yml#L21
There was a problem hiding this comment.
Sure, that could also work. The thing with hack is that its a fairly complex all-included utility, whereas what we need here is a really dumb way to extract an older version text, switch back to it, and just run regular tests. With hack, we don't know all the things it may do (or forget to do)... Not that important though, so up to you
There was a problem hiding this comment.
Yeah I don't really care either way; just curious 😂 and eventually this will get built into cargo.
Feel free to merge when the latest CI run succeeds.
| pub trait AsyncBackend { | ||
| /// Reads exactly `length` bytes starting at `offset` | ||
| async fn read_exact(&self, offset: usize, length: usize) -> PmtResult<Bytes>; | ||
| fn read_exact( | ||
| &self, | ||
| offset: usize, | ||
| length: usize, | ||
| ) -> impl Future<Output = PmtResult<Bytes>> + Send; | ||
|
|
||
| /// Reads up to `length` bytes starting at `offset`. | ||
| async fn read(&self, offset: usize, length: usize) -> PmtResult<Bytes>; | ||
| fn read(&self, offset: usize, length: usize) -> impl Future<Output = PmtResult<Bytes>> + Send; | ||
| } |
There was a problem hiding this comment.
Do we want to go all the way with trait-variant now (simplifies syntax and creates a variant for single-threaded executors that doesn't require a Send bound)? Or just implement the path we care about? I'm completely fine with the code as-is, but just raising the question to document and make sure it's a conscious decision.
Background: https://blog.rust-lang.org/2023/12/21/async-fn-rpit-in-traits.html#async-fn-in-public-traits
There was a problem hiding this comment.
Thx, good link. Technically, what I propose here is a direct replacement of the async_trait -- which in our case only implemented the Send version (it could have been both with #[async_trait(?Send)]). I would say YAGNI - unless someone asks for it?
Requires Rust v1.75 - old enough to release now
Also fixes a few pedantic clippy things and CI improvements