(breaking change) Add std and alloc features

[notgull]

These features do nothing and raise a compiler error if disabled. However this enables us to disable these features in the future.

[zeenix]

Firstly, why is this a breaking change? 🤔

These features do nothing and raise a compiler error if disabled. However this enables us to disable these features in the future.

While I understand the motivation here, having std listed as a supported feature will give folks the wrong idea and there is nothing worse than empty promises. This is made worse by the fact that list of features in docs.rs doesn't have a description. We can certainly document this well but AFAIK people often look at the feature section on docs.rs. We also need to be prepared for the possibility that nostd is never added to this repo for any reason.

There is no hurry on smol-rs/smol#31 so if you are confident that you can add the implementation soonish, we can hold off on 1.0 for now.

[notgull]

Firstly, why is this a breaking change? 🤔

If a crate has this in its Cargo.toml it will now fail to compile.

[dependencies]
async-broadcast = { version = "0.7", default-features = false }

They will need to change it to:

[dependencies]
async-broadcast = { version = "0.7", default-features = false, features = ["std"] }

While I understand the motivation here, having std listed as a supported feature will give folks the wrong idea and there is nothing worse than empty promises. This is made worse by the fact that list of features in docs.rs doesn't have a description.

I don't think this is an issue in practice: crossbeam-channel does this as well and I don't think users file issues about it.

We also need to be prepared for the possibility that nostd is never added to this repo for any reason.

As a counterpoint, we need to prepare for the possibility that it will.

This is largely about stability. If we release v1.0 now and then add a no-std impl later, we'll need to make another breaking change to make it no-std compatible.

[zeenix]

If a crate has this in its Cargo.toml it will now fail to compile.

Oh I missed the compiler error you inserted. Given we don't have any features currently, I think this is only breaking in theory.

I don't think this is an issue in practice

I disagree with the argument (once you realise that the feature is dummy, there is no reason to file an issue) but I think the compiler error you added, will prevent people from making a mistake. It's just very annoying to find out that way though.

As a counterpoint, we need to prepare for the possibility that it will.

Sure but since we don't have any features (let alone default ones), I don't think this will be a breaking change in practice.

[notgull]

Given we don't have any features currently, I think this is only breaking in theory.

No, this is breaking in practice. If anyone does import async-broadcast with no-default-features it will break their build.

[zeenix]

No, this is breaking in practice. If anyone does import async-broadcast with no-default-features it will break their build.

Yes but they won't and shouldn't because there are no features to disable. If people do this for no reason, of course their builds will break later. Should we really confuse others with a dummy feature for people who do that? I don't think so.

[zeenix]

I don't think so.

But if you strongly disagree and want to cater for people who disable default features out of habit or by mistake, I think it's easier to just wait till we have nostd implemented or we can decide not to go nostd in the near future. Those are the only choices IMO.

Btw I learnt about embassy_sync::pubsub::PubSubChannel yesterday. Might be good to check that out.

[zeenix]

Based on my previous comment, I'm closing this.

[notgull]

Yes but they won't and shouldn't because there are no features to disable. If people do this for no reason, of course their builds will break later. Should we really confuse others with a dummy feature for people who do that? I don't think so.

It doesn't matter. Theoretically there could be a user of async-broadcast that imports it as no-default-features. If an std feature is added that removes functionality and they import it, it will break their build.

This strategy is used by other crates in smol-rs by the way.

[zeenix]

Theoretically there could be a user of async-broadcast that imports it as no-default-features

"theoretically", yes. That's my whole point here. Even if there actually are any users who do that (for some unknown reason), I don't think we need to cater for them.

This strategy is used by other crates in smol-rs by the way.

Maybe they shouldn't. :)