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

With the move of the Error Trait into core, the std flag is no longer needed. #100

Open
juliankrieger opened this issue Oct 20, 2024 · 2 comments

Comments

@juliankrieger
Copy link

juliankrieger commented Oct 20, 2024

I've opened a PR here: #101

With Rust 1.81, the error trait now lives in core. I think coset can live with no_std and alloc only if we reflect that change.

@daviddrysdale
Copy link
Collaborator

Thanks!

The code change looks fine, but there would need to be some knock-on effects:

  • Presumably this makes 1.81 the MSRV for the crate? (Currently it's 1.58)
  • Removal of a feature is going to need a major version bump.

So we'll probably wait for a while before merging this PR.

@juliankrieger
Copy link
Author

juliankrieger commented Oct 21, 2024

There are a couple of strategies you could employ to release this earlier.

  • You could feature gate the trait impl behind a rust-1.81 feature
  • You could feature gate the trait impl behind a v.1.81 specific feature from https://crates.io/crates/rustversion
  • Re-export the Error Trait and change the MRSV to 1.81 only for no_std environments
  • Introduce a rustc flag similar to anyhow (see core::error, no_std dtolnay/thiserror#304 (comment))

Meanwhile, you could avoid breaking changes by keeping the std feature in the crate, but not apply any cfg attributes that depend on it. I actually use this strategy for all my no_std-by-default crates so that I can avoid a breaking change in the future if I need something from std.

Of course, waiting for Rust v.1.81 to mature is a fine strategy as well. With https://github.com/esp-rs/rust being the only active Rust fork I know also recently hitting 1.81, I would be very interested in hearing your opinion on why keeping the MRSV that low is useful!

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

No branches or pull requests

2 participants