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

Default mmap feature is described as non-default in Cargo.toml (UPDATE: reverted in v1.6.1, comment on this issue if this breaks you) #452

Open
ArmoredPony opened this issue Feb 27, 2025 · 6 comments

Comments

@ArmoredPony
Copy link

In commit a12fa7b mmap feature was added to the default feature list in Cargo.toml, but its description says that this feature is disabled by default. Should the description be changed?

oconnor663 added a commit that referenced this issue Feb 27, 2025
I committed this change accidentally in
a12fa7b. Thanks to @ArmoredPony for
catching this in #452.
@oconnor663
Copy link
Member

Damn, I committed that line by mistake. (I need to figure out a better way to tell rust-analyzer to turn on features in my editor.) I've pushed a commit to master that reverts it. I'm probably going to release this today as v1.6.1, even though it's technically a backwards-incompatible change. The bug is only a week old, anyone who relied on it this week can easily add the feature, and releasing v2.0.0 for this seems ridiculous. What do you think?

@ArmoredPony
Copy link
Author

God I hate SemVer. I'd release it as 1.6.1, but I've never had such big and popular repos like this, so that would be a bad advice. I think that you should stick to SemVer, comply with guidelines, and bump the major version

It kills me that semver check is not a part of default cargo util set despite being out there for 2 years

@oconnor663
Copy link
Member

The intended approach (adding the mmap feature if you use mmap functions) is documented and compatible with all released versions, so I'm ok taking the risk. Will push it out after CI passes.

@oconnor663 oconnor663 changed the title Default mmap feature is described as non-default in Cargo.toml Default mmap feature is described as non-default in Cargo.toml (UPDATE: reverted in v1.6.1, comment on this issue if this breaks you) Feb 27, 2025
@oconnor663
Copy link
Member

Released v1.6.1.

@BurningEnlightenment
Copy link
Collaborator

In such cases I like to cite Hyrum's Law:

With a sufficient number of users of an API,
it does not matter what you promise in the contract:
all observable behaviors of your system
will be depended on by somebody.

Given that observable behavior might include code size for some people on embedded systems, it is almost (?) impossible to release something truly backwards compatible. IMHO versioning is all about communicating intent and hedging the risks for users. Just my 2 cents.

@ArmoredPony
Copy link
Author

maybe you should yank the SemVer breaking version

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

3 participants