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

Remove deprecated functionality #731

Merged
merged 1 commit into from
Feb 5, 2018
Merged

Remove deprecated functionality #731

merged 1 commit into from
Feb 5, 2018

Conversation

cramertj
Copy link
Member

@cramertj cramertj commented Feb 5, 2018

Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

Looks great! I'm sure we'll have more cleanup on the way but feel free to land this whenever you're ready.

@@ -58,7 +57,6 @@ fn with<F: FnOnce(&BorrowedTask) -> R, R>(f: F) -> R {
pub struct Task {
id: usize,
Copy link
Member

Choose a reason for hiding this comment

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

Weird I thought this was deprecated but we may still be using it somewhere? Can always figure that out later of course.

Copy link
Member

Choose a reason for hiding this comment

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

We never deprecated the actual id but all usage of it has been deprecated.

The id should be removed in 0.2.

Copy link
Member Author

Choose a reason for hiding this comment

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

Isn't id still used in calls to with_notify, so that it will be passed down into the inner call to notify?

Copy link
Member

Choose a reason for hiding this comment

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

It is indeed! That's, confusingly, a different id though, which indeed can't be removed unless we remove the ids from the Notify trait.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, gotcha. Removed.

use super::core;
use super::{BorrowedTask, NotifyHandle, Spawn, spawn, Notify, UnsafeNotify};
pub use super::core::{BorrowedUnpark, TaskUnpark};
Copy link
Member

Choose a reason for hiding this comment

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

Now that there's no need for unpark events this can (in the future of course if you'd prefer) be cleaned up a lot and not have this sd/core business. These types can move from the core module directly into the parent module here to directly make up Task and remove a few layers of indirection in the meantime.

Cargo.toml Outdated
@@ -22,8 +22,7 @@ appveyor = { repository = "alexcrichton/futures-rs" }

[features]
use_std = []
Copy link
Member

Choose a reason for hiding this comment

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

While we're at it I think the conventional name for this today is just std, so we could change that here 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.

Done.

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