-
Notifications
You must be signed in to change notification settings - Fork 440
Expand editions docs for what is considered for inclusion in an edition #28197
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
base: main
Are you sure you want to change the base?
Conversation
…new features Signed-off-by: Jade Abraham <[email protected]>
|
Please consider this a rough draft, but I would like to get input from the people originally involved in the design of editions to ensure I am accurately representing what we reached consensus on (@lydia-duncan, @e-kayrakli, @ShreyasKhandekar, and @mppf) Also pinging @bradcray for his attention due to his interest in the topic. |
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
lydia-duncan
left a comment
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.
For the most part, the proposed document as it stands matches my understanding of our final conclusions, but I have thumbs-up'd a couple of Michael's comments. I would have to go digging again through our notes and communication out to the team to determine which of Michael's comments that I disagree with are due to faulty memory on my part - I may still do that, but I've got other stuff vying for my attention at the moment
Signed-off-by: Jade Abraham <[email protected]>
|
I think I've addressed the current set of feedback (thank you!), except where the conversation is not yet marked resolved |
Signed-off-by: Jade Abraham <[email protected]>
ShreyasKhandekar
left a comment
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.
I have a small point about linking to the deprecation policy, but this looks good to me otherwise.
Signed-off-by: Jade Abraham <[email protected]>
e-kayrakli
left a comment
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.
Looks good. I made some inline comments, none requires holding up the merge IMO.
|
|
||
| A stable feature should only be removed in the ``preview`` edition. The | ||
| ``preview`` edition collects changes that will be included in the next | ||
| edition. The act of marking a stable feature as removed in a future edition |
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.
Mild preference for dropping the "The preview edition collects" sentence as you said it above. I feel like this is a piece of doc that would be read linearly rather than jumping around.
|
|
||
| In summary, if a user is using only stable features of a given language | ||
| edition, two different versions of the compiler that support that edition | ||
| should behave the same way (barring bug fixes, performance improvements, etc). |
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.
Maybe just barring bugs or barring bugs, or lack thereof. Performance improvement feels irrelevant
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.
And we all know that "behave" here is not well defined :)
Expands the documentation for editions to include information about what kinds of changes actually belong in an edition and how they should be added to an edition.
I consider this PR to subsume and resolve #27096, since all of the information in that issue should now be in the docs.