Split up callback and future/stream APIs.

[Aehmlo]

Closes #36. The future/stream versions are placed behind the futures feature gate. This gate is propagated through the workspace using appropriate feature names (use_futures). Submodule names were taken from 6c20c5c (not from the issue text of #36, as the two are different).

I understand the importance of establishing a firm precedent, so I'm prepared to undergo some bikeshedding here. I just happened across the repo and figured I'd check something easy off the to-do list. :)

Outstanding questions that left me slightly uncertain:

• Do we like futures as a feature name at the workspace level? Do we even want to have something at the workspace level?
• Relatedly, is use_futures an acceptable feature name in the absence of namespaced features?

[fitzgen]

Looks good to me, thanks @Aehmlo!

---

I don't have qualms about naming the feature "futures", since many crates have a "serde" feature to enable serialization APIs and also depend on the serde crate. Don't feel super strongly, though. @Pauan?

[Pauan]

@fitzgen Features and crates go into the same namespace, so by using the futures name, we make it impossible to also have a futures feature:

[dependencies]
futures = { version = "0.1.25", optional = true }

[features]
futures = ["futures", "foo"]

That gives the following error:

error: failed to parse manifest at `Cargo.toml`

Caused by:
  Features and dependencies cannot have the same name: `futures`

Using the same name for the feature + crate only works if you only want to enable that one crate. But if you want to enable multiple crates, you must use a different name.

A good example of this would be wanting to enable serde and serde_derive and serde_json with a single feature. In that case you cannot call the feature serde.

So if you start off with a serde feature, and then later realize you want to extend it to multiple crates, you cannot do so in a backwards-compatible way.

That means that (even though it's common) all of the crates which have a serde or futures feature are doing it wrong.

Personally, I think it's really bad that Cargo puts features and crates into the same namespace (why?!), but that's how it is.

[Aehmlo]

@Pauan There's work underway to fix this issue (see rust-lang/cargo#5565). This is, however, what I was referencing above, and why the feature name in the crate that actually depends on futures is use_futures — I'm just not sure if this is the final feature name we want to use. I assume we want to pick something now and stick with it for everything in this project.

[Pauan]

@Aehmlo Oh, it's good to know that it'll be fixed (eventually?)

I have no problem with use_futures (or other suggestions like gloo-futures or futures-support, etc.)

But the current proposal is to use futures for both the crate name and the feature, which I disagree with, hence my comment to @fitzgen

[OddCoincidence]

Couldn't this be worked around by aliasing the package to futures_rs or something?

futures_rs = { version = "0.1.25", package = "futures", optional = true }

[Pauan]

@OddCoincidence I just tested it, and it seems to work!

So assuming we're all okay with renaming packages like that, I'm okay with using futures as a feature name.

[Aehmlo]

This is news to me; thanks, @OddCoincidence! I’ll go ahead and make this change later today.

[Aehmlo]

I've changed the feature name to be futures everywhere, using extern crate futures_rs as futures; so people aren't surprised when futures imports don't work. I've hidden this line in the doc comments, since it's very much an implementation detail. I'm very happy with this final solution; thanks all!

[fitzgen]

I asked @Pauan in discord:

do you have an opinion on the futures feature name? making all the futures dependencies funky is a bit... unideal. but I also don't care too much

and @Pauan replied:

my opinion is that the funkiness is okay, since it's internal to the library
whereas the feature is external (i.e. consumers will use it)
so making things better for consumers at the expense of a bit of funkiness internally is okay

I think this is a great point.

Let's move forward with renaming the futures dependency in Cargo.toml, and exposing the feature name "futures".

[fitzgen]

Looks great! Let's merge this with a note for users in the docs about the feature. Thanks @Aehmlo!

[fitzgen]

Great -- thanks! We can workshop wording in follow up PRs if necessary.