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

Semver: Note that it is not a breaking change to make an unsafe function safe #11200

Closed
wants to merge 3 commits into from

Conversation

madsmtm
Copy link
Contributor

@madsmtm madsmtm commented Oct 9, 2022

I asked this a while ago on the users forum, and thought it would be nice to have in the main documentation.

I marked this as a "minor" change because the only way it breaks is if the user is opting out of Rust's stability guarantees in general. But happy to make it "possibly breaking" if that suits you?

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 9, 2022
Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

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

Thanks!

I think at some point in the future, we'll also want to cover unsafe traits/impls.

src/doc/src/reference/semver.md Outdated Show resolved Hide resolved
src/doc/src/reference/semver.md Outdated Show resolved Hide resolved
madsmtm and others added 2 commits October 12, 2022 21:48
Co-authored-by: Eric Huss <eric@huss.org>
@madsmtm
Copy link
Contributor Author

madsmtm commented Oct 12, 2022

Sorry for the delay, I think I've resolved your concerns now.

I agree about traits, and also wanted to add a section about const fn, but wanted to keep the PR small.

@ehuss
Copy link
Contributor

ehuss commented Feb 24, 2023

Per #11596, we've decided that anything that introduces a new lint is a minor change. Would you be willing to update the text here to change this to "Minor"? Of course it would be good to point out the potential hazard, but we aren't considering those as breaking changes.

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 24, 2023
bors added a commit that referenced this pull request May 9, 2023
Semver: Note that it is not a breaking change to make an unsafe function safe

This is a repost of #11200 with some requested edits made. This makes it clear that it is a minor change due to our policy that triggering new lints is not a breaking change. I also simplified it by not repeating what constitutes a breaking change for a trait definition, and instead link to the rule that specifies no signature changes are allowed.
@bors
Copy link
Contributor

bors commented May 9, 2023

☔ The latest upstream changes (presumably #12116) made this pull request unmergeable. Please resolve the merge conflicts.

@ehuss
Copy link
Contributor

ehuss commented May 9, 2023

Closing as merged in #12116.

@ehuss ehuss closed this May 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants