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 std #85

Merged
merged 3 commits into from
Sep 10, 2024
Merged

Remove std #85

merged 3 commits into from
Sep 10, 2024

Conversation

Christiaan676
Copy link
Contributor

@Christiaan676 Christiaan676 commented Sep 10, 2024

Changes to:

  • Switch to core::error::Error
  • Remove std feature
  • Fixed two Clippy warnings

As requested in #74

Changes do bump the MSRV to 1.81 as core::error was stabilized in this release.

@tarcieri tarcieri merged commit 49dfead into RustCrypto:master Sep 10, 2024
14 checks passed
Copy link
Member

@newpavlov newpavlov left a comment

Choose a reason for hiding this comment

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

The MSRV bump is a viral change which will affect almost all our crates. Are we sure that we want such drastic bump? After all, it's the latest stable released only several days ago.

Personally, I am on the fence. It's certainly nice to be able to remove the std feature from many crates and it may allow us to use some recently stabilized features, but the MSRV bump is a significant price to pay for that which is likely to affect a lot of our users.

@@ -14,14 +14,13 @@ categories = ["no-std", "data-structures"]
keywords = ["generic-array"]
readme = "README.md"
edition = "2021"
rust-version = "1.65"
rust-version = "1.81.0"
Copy link
Member

Choose a reason for hiding this comment

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

I think it's better to use just 1.81 here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately the documentation on rust-version only mentions that two or three components is allowed. Not if they are handled differently.

Can you explain why the two components version is better? (if needed I can create a MR to change it)

Copy link
Member

@newpavlov newpavlov Sep 11, 2024

Choose a reason for hiding this comment

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

It's the same logic as with skipping zero minor/patch versions in dependencies (e.g. we use zeroize = "1.8", not zeroize = "1.8.0"). The notations mean the same thing, but 1.81 is shorter and easier to read. I think it can be updated in a pre-release PR.

@tarcieri
Copy link
Member

tarcieri commented Sep 10, 2024

@newpavlov I think we should generally get this change into the next set of crates, even if it's an aggressive MSRV bump. If people don't want the bump, they can stick with the last stable series of releases.

I am down to hold off on releasing crates until such a time as this seems less drastic than it is today. Generally the ecosystem seems to have converged on stable - 3 as a satisfactory release window for people who aren't sticking with the antiquated "whatever Debian stable is" window (IIRC Rust 1.63, but speaking as a Debian stable user that's far too old to be useful for me and I install via rustup).

That would put the release timeframe around December 2024.

@tarcieri
Copy link
Member

Here's a PR to fix the version: #87

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