Cargo.toml: Deprecate executor specific features for sub-crates#2962
Cargo.toml: Deprecate executor specific features for sub-crates#2962thomaseizinger merged 8 commits intomasterfrom
Conversation
We only want to emit the deprecation warning in case a user uses the old features and _not_ the new ones. This assumes that a user upgrades properly, i.e. adds the new `tokio` or `async-std` feature AND removes the old `tcp-tokio` feature from the list. We need to use this rather complex cfg expression because otherwise, our CI won't pass. We run clippy with `--all-features`, thus just detecting for presence of the `tcp-async-io` feature for example will always flag our examples as using deprecated code.
Otherwise CI will not pass once #2963 actually checks it properly.
|
In favor of this! |
The annoying thing with removing features is that The logic is a bit complex but it is only temporary until the release is out. We can remove it immediately after so I wouldn't be too worried about it :) |
mxinden
left a comment
mxinden
left a comment
There was a problem hiding this comment.
I am in favor the change in general.
I am in favor of the two step (deprecation + removal) process. While not directly related, I think #2971 is a good data point for the argument that configuring rust-libp2p features is hard. Thus I think making it easier for users at the expense of complexity on our end makes sense.
Before we merge here, can you create a second pull request removing the deprecated modules. Once this is merged and released, we can then merge the second pull request.
Done here: #3001 |
3 participants
Description
Typically, an application will only use one kind of executor like
tokioorasync-std. Currently, users of libp2p need to activate a separate feature for each sub-crate that is specific to the executor they want to use, i.e.tcp-tokio,dns-tokio, etc.With this PR, we deprecate these features in favor of a single
tokioandasync-stdfeature. Thanks to cargo's support for weak-features, we can forward the respective executor feature to each sub-crate without pulling them in as dependencies unless the user also activated the corresponding protocol (dns,tcpetc).Deprecation warnings don't work on re-exports so I had to (temporarily) define a new module in
lib.rs. This module should be removed again once we actually remove the deprecated features.Here is a screenshot of the warning being active:
This is a backwards-compatible change.
The warning goes away as soon as we stop using the deprecated feature:
Links to any relevant issues
Open Questions
Change checklist
I have added tests that prove my fix is effective or that my feature works