-
Notifications
You must be signed in to change notification settings - Fork 50
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
Bump tokio to version "1" #165
Conversation
Opting to only specify major version to limit dependency maintenance.
@@ -27,7 +27,7 @@ rand = { version = "0.8.5", features = ["small_rng"] } | |||
rand_distr = "0.4.3" | |||
regex = { version = "1", optional = true } | |||
scoped-tls = "1.0.1" | |||
tokio = { version = "1.25.0", features = ["full"] } | |||
tokio = { version = "1", features = ["full"] } |
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.
Can it work with "earlier" Tokio versions? If not, keeping this set to the minimum version that it can work with is usually best. Can be checked with --minimal-versions
.
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.
I don't have nightly. Is there a stable way to check this?
I actually looked at hyper
before changing this. Is hyper >= 1.0.0 only?
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.
hyper only depends on some methods that existed at 1.0. I think Tokio needed to add something for turmoil, no? In that case, you'd need that version as the minimum one here.
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.
There is nothing within tokio that knows about turmoil. turmoil simply uses tokio constructs and mirros the net type apis.
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.
Oh I didn't mean it knows about turmoil, but that something was missing before turmoil could exist. Maybe I'm wrong.
You could also add a minimal versions job to the CI. Or if you're certain there's nothing, we can just ship. Up to you.
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.
I see. So we use unhandled_panic
, which is newer than 1.0.0. I think I'll cancel this for now. Thanks for the review!
Cancelling. |
Opting to only specify major version to limit dependency maintenance.