-
Notifications
You must be signed in to change notification settings - Fork 39
chores: Bump smol version to 2.0 #221
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
base: master
Are you sure you want to change the base?
Conversation
This should fix Restioson#220. The changes include removing smol-potat in favor of smol::block_on. However, this commit does not remove smol-timeout as it is heavily used on tests and the crate seems to be unmaintained. Nontheless tests keep working.
|
Hey, thanks for the PR! Really sorry for getting back to this so late, studies have been all-consuming this year. I wonder if we would want to keep smol 1.0 support under a different feature (e.g. smol-1)? |
As long as there is a release with smol-1 support, I wouldn't bother too much :) |
|
That's fair, since I suppose we haven't really made any other changes |
|
So, almost a year later, I noticed this PR was still open and that I missed your review. Sorry for that. I ran |
|
Apologies - this dropped off my radar too! |
|
Hmm, looks like a build failure - I think smol relies on async_task? |
|
I think it had something to do with Cargo.lock being modified in both branches so it was stuck in an invalid state. I recreated the lock file with the latest dependencies. I would advise removing the lockfile, as it mostly does nothing for libraries, and switch to something like https://crates.io/crates/cargo-minimal-versions to validate the minimum requirements. |
Having the lockfile ensures deterministic CI runs, which is important in the presence of e.g. semver-incompatible bumps etc It is also the official recommendation from the Rust folks: https://blog.rust-lang.org/2023/08/29/committing-lockfiles/ |
Cargo.lock
Outdated
| [[package]] | ||
| name = "async-std" | ||
| version = "1.12.0" | ||
| version = "1.13.1" |
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.
Why is async-std getting bumped when we are only changing smol? 🤔
|
The diff on the lockfile seems a bit excessive to me. Can you checkout the one from main and redo the bump to make that smaller? Thanks! |
Oh, I guess I was using the old guidelines. Thanks for sharing.
Done. I had to use Rust 1.75 as 1.88 was failing for wasm_bindgen with the following error: |
|
LGTM, happy for you to merge @thomaseizinger when you're happy |
|
Is the CI error reproducable locally? |
This should fix #220. The changes include removing smol-potat in favor of smol::block_on. However, this commit does not remove nor update smol-timeout as it is heavily used on tests and the crate seems to be unmaintained. Nonetheless tests keep working.