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

Make ThreadPool optional #1910

Merged
merged 1 commit into from Oct 11, 2019
Merged

Conversation

taiki-e
Copy link
Member

@taiki-e taiki-e commented Oct 11, 2019

This makes ThreadPool optional (disabled by default).
cc #1893

Closes #1724

r? @cramertj
cc @seanmonstar

@cramertj cramertj merged commit a3e5962 into rust-lang:master Oct 11, 2019
1 check passed
@taiki-e taiki-e deleted the executor-threadpool branch Oct 12, 2019
@Matthias247
Copy link
Contributor

Matthias247 commented Oct 27, 2019

This seems to have broken unit-tests with default features for me:

cargo test yields:

   Compiling futures-preview v0.3.0-alpha.19 (C:\Users\matth\Code\rust\futures-rs\futures)
error[E0432]: unresolved import `futures::executor::ThreadPool`
 --> futures\tests\eventual.rs:2:5
  |
2 | use futures::executor::ThreadPool;
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `ThreadPool` in `executor`

error[E0433]: failed to resolve: could not find `ThreadPool` in `executor`
  --> futures\tests\mutex.rs:39:39
   |
39 |     let mut pool = futures::executor::ThreadPool::builder()
   |                                       ^^^^^^^^^^ could not find `ThreadPool` in `executor`

Is there something missing, that it gets enabled for test builds again?

@taiki-e
Copy link
Member Author

taiki-e commented Oct 30, 2019

We only test with --all-features, so it is recommended to pass --all-features flag when running tests locally.
https://github.com/rust-lang-nursery/futures-rs/blob/99d5ebed1288fe61e21151c86b2223e8872070c0/.travis.yml#L200-L201

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.

3 participants