Skip to content
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

default all feature flags to off #1811

Merged
merged 11 commits into from
Nov 22, 2019
Merged

default all feature flags to off #1811

merged 11 commits into from
Nov 22, 2019

Conversation

carllerche
Copy link
Member

Changes the set of default feature flags to []. By default, only
core traits are included without specifying feature flags. This makes it
easier for users to pick the components they need.

For convenience, a full feature flag is included that includes all
components.

Tests are configured to require the full feature. Testing individual
feature flags will need to be moved to a separate crate.

Closes #1791

Changes the set of `default` feature flags to `[]`. By default, only
core traits are included without specifying feature flags. This makes it
easier for users to pick the components they need.

For convenience, a `full` feature flag is included that includes all
components.

Tests are configured to require the `full` feature. Testing individual
feature flags will need to be moved to a separate crate.

Closes #1791
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we've determined that this is the right thing to do, LGTM. I think including a "full" feature and making sure that all the documentation prominently states that it is the recommended way to get started is important, to avoid confusing new users.

tokio/src/lib.rs Show resolved Hide resolved
Copy link
Member

@olix0r olix0r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

tokio/Cargo.toml Outdated Show resolved Hide resolved
@davidbarsky
Copy link
Member

README.md structure:

### Tokio for Applications

`tokio = { version = "0.2, features = ["full"] }`

### Tokio for Libraries

`tokio = { version = "0.2, features = ["features"] }`

Each of the features should be pretty-well documented.

@@ -3,6 +3,7 @@ pr: ["master"]

variables:
RUSTFLAGS: -Dwarnings
nightly: 2019-11-16
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why what?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we now pinning the nightly?

@@ -1,5 +1,6 @@
#![cfg(unix)]
#![cfg(feature = "full")]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line should be moved down by one to be consistent with all the others.

@@ -0,0 +1,19 @@
macro_rules! cfg_codec {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does tokio-util need these helpers when none of the other crates (afaict) do?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tokio needs the helpers too?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH, tokio-util is kind of under developed ATM. We may want it to be tokio-codec 🤷

@carllerche carllerche merged commit 7b4c999 into master Nov 22, 2019
@carllerche carllerche deleted the features-default-off branch November 28, 2019 04:10
@Menschenkindlein
Copy link

Hi! I'd like to ask you to revise this decision. It looks like it brings more problems for newcomers than benefits for powerusers (#1833). Cargo already has a tool for disabling default features: default-features = false. As the ability to cherry-pick features is mostly needed for advanced users, advanced workflows should be reserved for them.

@Darksonn
Copy link
Contributor

Having frequented both the user's forum and this issue tracker, this is not something people ask about often. Adding it to the README.md as you suggested on the other issue is fine if it's missing, but having a full feature like we do now has quite effectively resolved lots issues where libraries just do tokio = "0.2" in their [dependencies] section, making it impossible for users of that library to disable any of Tokio's features.

Libraries depending on too many features of other libraries is incredibly common for projects with several default features.

@Menschenkindlein
Copy link

Menschenkindlein commented Jun 22, 2020

Thank you for the clarification! I don't like the idea of fixing other libraries by putting obstacles for a quick start, but good documentation will really help to resolve this problem. Everyone would learn about feature flags at some point in time. It won't hurt if Tokio is the crate that requires this knowledge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider defaulting Tokio features to off instead of on.
8 participants