test against rust stable/nightly versions #14
Conversation
|
@kenkoooo -san Would it be possible to get it merged? Seems like I lost admin role here, your help is very much appreciated 🙏 |
.github/workflows/ci.yml
Outdated
| @@ -15,7 +15,7 @@ jobs: | |||
| fail-fast: false | |||
| matrix: | |||
| os: [ubuntu-latest, macos-latest, windows-latest] | |||
| version: [1.68, stable] | |||
| version: [stable, nightly] | |||
There was a problem hiding this comment.
I completely agree with you that we should run tests not only on the "nightly" version but also on the "stable" version. If we want to support version 1.68 as the Minimum Supported Rust Version (MSRV), it would be best to include it in the CI.
| version: [stable, nightly] | |
| version: [1.68, stable, nightly] |
There was a problem hiding this comment.
@kenkoooo san
Thank you! As I stated above, it causes dependency version conflicts. (in particular, tokio in this case)
Maybe we could lock the dependency versions only for 1.68 testing, and use the latest crates for the stable/nightly, (some changes are required with CI matrixes settings..?)
There was a problem hiding this comment.
Okay, we no longer support version 1.68. In that case, we might want to set another version as the MSRV. Which version should we specify as the new MSRV?
There was a problem hiding this comment.
I think there are two options:
- Specify version 1.70.0 as the new MSRV, which aligns with the latest version of tokio (1.39.2).
- Use tokio version 1.38.1, whose MSRV is 1.63.0. Since we use it only for testing, this might not be a problem.
There was a problem hiding this comment.
@kenkoooo-san
Thanks for your advices!
I'm not very keen on updating MSRV for testing reason - we might have to update our CI everytime when tokio 1.xx changes its MSRV support. (which looks quite frequent).
I thought it should be nice to specify tokio 1.38.1 for MSRV testing only, while using the latest minor versions of dependencies for the latest stable rust. Would that make sense to you..?
There was a problem hiding this comment.
(it seems I'm not having maintainership of this crate any more, so it would be great if you can approved it to get it merged. Thank you in advance..!)
fursich
force-pushed
the
chore/ci-rust-versions
branch
from
2ecabbd to
7796ce9
Compare
.github/workflows/ci.yml
Outdated
| # tokio 1.39+ requires rust <= 1.70.0 | ||
| # https://github.com/tokio-rs/tokio?tab=readme-ov-file#supported-rust-versions | ||
| - run: cargo update -p tokio --precise 1.38.1 | ||
| if: matrix.version == '1.68.0' |
There was a problem hiding this comment.
We might want to consider updating the dev-dependencies to tokio = "=1.38.1" rather than updating the workflow. I feel that we don’t need to use the latest tokio in the stable version either, since it’s only used for testing.
There was a problem hiding this comment.
Thank you. I understand that's one of the suggestions you gave me earlier. I thought this crate is most likely to be used with tokio (for async loading) in real case anyway, and it might be nicer to test against arbitrary (latest) versions of tokio as well - what would you think?
There was a problem hiding this comment.
Let’s focus on what we want to ensure: that our async functions work as expected on any async runtime. We don’t need to prioritize compatibility with the latest version of tokio.
There was a problem hiding this comment.
Ok, that's a fair point. Thank you for your feedback and handling 🙏
Our CI used to fix a (relatively old) version of rust to test against in order to make sure this crate can be used with any newer versions, but it causes sudden failure of CI when dependencies are updated, which is not nice to new contributors especially.
This PR is to test against the latest stable version, plus nightly only (Testing nightly may seem too much, so we could omit them if CI frequently fails)