-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[book] add chapter on breaking changes #7831
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for getting this started!
I think there's some stuff missing here. It seems like there are a lot of changes to item signatures not covered. Another is attributes. I'd also like to highlight some of the "grey area" things where not everyone agrees if it is a breaking change or not. One in particular is rust version support, and also platform requirements (like a new release requires a new version of macOS, or a newer system library, or anything really). I'm not sure if we need to exhaustively hit everything in this PR, but any help working through them would be appreciated.
@@ -0,0 +1 @@ | |||
# Breaking Changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a stray file?
2. **Minor**: incremented for backwards-compatible feature additions. | ||
3. **Patch**: incremented for backwards-compatible bug fixes. | ||
|
||
> **Note** As well as the rule that versions **<1.0.0** can change anything. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't quite correct. Cargo treats 0.y.z where changes in z are compatible.
> **Note** As well as the rule that versions **<1.0.0** can change anything. | ||
|
||
Typically, a **breaking change** is a change that, *strictly speaking*, can cause downstream | ||
code to fail to compile |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code to fail to compile | |
code to fail to compile. |
@@ -21,6 +21,7 @@ | |||
* [Cargo Reference](reference/index.md) | |||
* [Specifying Dependencies](reference/specifying-dependencies.md) | |||
* [Overriding Dependencies](reference/overriding-dependencies.md) | |||
* [Breaking Changes](reference/breaking.md) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mind moving this down a bit? The Manifest Format
is one of the most important chapters, and I'd like it to be near the top. Maybe down near the "Publishing" chapter, since this is kinda related to publishing.
I'm also wondering if maybe it should be placed in a sub-folder, and breaking it up a little? The chapter is a bit difficult to navigate (or see an overview).
I'm also thinking maybe naming it "Version Compatibility"? Or "SemVer Compatibility"?
(The use of `?Sized` is essential; otherwise you couldn't recover the original | ||
signature). | ||
|
||
[rfc]: https://github.com/rust-lang/rfcs/pull/1105 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[rfc]: https://github.com/rust-lang/rfcs/pull/1105 | |
[rfc]: https://github.com/rust-lang/rfcs/blob/master/text/1105-api-evolution.md |
impl, which has the wrong type. | ||
|
||
Once more, this is considered a minor change, since UFCS can disambiguate (see | ||
"Adding a defaulted item" above). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Link to section.
And maybe add <a>
tags to create shorter and more stable url fragments?
It's worth noting, however, that if the signatures *did* happen to match then | ||
the change would no longer cause a compilation error, but might silently change | ||
runtime behavior. The case where the same method for the same type has | ||
meaningfully different behavior is considered unlikely enough that the RFC is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"RFC"
|
||
**Breaking changes are assumed to be major changes unless otherwise stated**. | ||
The RFC covers many, but not all breaking changes that are major; it covers | ||
*all* breaking changes that are considered minor. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit of a strong statement, I'm not sure if it is accurate since I don't consider this list to be exhaustive.
[ufcs]: https://doc.rust-lang.org/1.7.0/book/ufcs.html | ||
[semver]: https://semver.org/ | ||
[features]: http://doc.crates.io/manifest.html#the-[features]-section | ||
[opt-in_features]: http://doc.crates.io/manifest.html#the-[features]-section |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[opt-in_features]: http://doc.crates.io/manifest.html#the-[features]-section |
[semver]: https://semver.org/ | ||
[features]: http://doc.crates.io/manifest.html#the-[features]-section | ||
[opt-in_features]: http://doc.crates.io/manifest.html#the-[features]-section | ||
[postponed_RFC]: https://github.com/rust-lang/rfcs/pull/757 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[postponed_RFC]: https://github.com/rust-lang/rfcs/pull/757 |
@ehuss I'm willing to help out on the missing pieces! I've noticed that other books in the rust ecosystem has TODO's, will that suffice here, so that extensions can be added in later pr's? |
I think writing them down in a tracking issue on this repo would be fine. |
@guswynn Was wondering if you're still interested in working on this. I might have a little extra time, so I was thinking of moving it forward. |
I may be able to find time to work on in this weekend, can I put it on hold
for 2 more days, after that you can take it?
…On Fri, May 15, 2020, 7:29 AM Eric Huss ***@***.***> wrote:
@guswynn <https://github.com/guswynn> Was wondering if you're still
interested in working on this. I might have a little extra time, so I was
thinking of moving it forward.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#7831 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABJHND2FAG3TQBFIUWRC5ALRRVGUZANCNFSM4KLTTIHA>
.
|
Sounds good! |
I'm going to close, as this is now incorporated in the book: https://doc.rust-lang.org/nightly/cargo/reference/semver.html |
@ehuss I'm so sorry, I ended up having a bout of time where I couldn't spend any time on this! |
As far as I can tell, rust-lang/rfcs#1105 are the de-facto guidelines that crate maintainers should use to guide their use of semver for this crates.
I have found it confusing in the past that these guidelines don't exist in any documentation outside that RFC, and discussion in rust-lang/rust#68004 led be to believe that it's worth it to add this to some official documentation.
See rust-lang/reference#738 for more details
@ehuss I only added the chapter on breaking changes, not the