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.

Sign up for GitHub

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

Jump to bottom

Amend the template from RFC 1589 #2658

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Amend the template from RFC 1589 #2658

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fe67169
Amend the template from text of RFC 1589 to match current actual
pnkfelix Mar 8, 2019
fcf1781
Address review feedback by putting back the initial paragraph of temp…
pnkfelix Mar 11, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 21 additions & 13 deletions text/1589-rustc-bug-fix-procedure.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,23 +104,27 @@ this page is describe why this change was made and how you can fix
code that is affected by it. It also provides a place to ask questions
or register a complaint if you feel the change should not be made. For
more information on the policy around future-compatibility warnings,
see our [breaking change policy guidelines][guidelines].
pnkfelix marked this conversation as resolved.
Show resolved Hide resolved
see our [breaking change policy guidelines][RFC 1589].

[guidelines]: LINK_TO_THIS_RFC
[RFC 1589]: https://github.com/rust-lang/rfcs/blob/master/text/1589-rustc-bug-fix-procedure.md

#### What is the warning for?
#### What is this lint about
pnkfelix marked this conversation as resolved.
Show resolved Hide resolved

*Describe the conditions that trigger the warning and how they can be
fixed. Also explain why the change was made.**
*Describe here what kinds of Rust code will cause the lint to trigger.*

#### When will this warning become a hard error?

At the beginning of each 6-week release cycle, the Rust compiler team
will review the set of outstanding future compatibility warnings and
nominate some of them for **Final Comment Period**. Toward the end of
the cycle, we will review any comments and make a final determination
whether to convert the warning into a hard error or remove it
entirely.
*Motivational text or historical information is also of use, but the
most important thing is to explain what kind of code the lint is
detecting.*

#### How to fix this warning/error

*Explain here what the developer needs to do to address the warning.*

#### Current status
Copy link
Contributor

Choose a reason for hiding this comment

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

On the other hand, this seems like an improvement, but perhaps a bit premature -- I'm not sure if we always take these three steps? Still, I like the idea of showing the process more graphically, and I think we should adopt a "standard" time frame that is tighter than the "open-ended process" suggested by the old template.

Copy link
Contributor

Choose a reason for hiding this comment

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

We probably don't always take these steps but I think we often do and it seems like a good procedure :) -- I agree with standardizing the time-frame, or at the very least having a time-frame at all.

Copy link
Member Author

Choose a reason for hiding this comment

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

So is there actually something for me to do here? I think starting with the provided three steps in the template makes more sense than leaving them out.

Do you want me to add an explicit note saying that people have the option to revise that section on a case-by-case basis?


- [ ] PR ? introduces the `YOUR_LINT_NAME_HERE` lint as warn-by-default
- [ ] PR ? makes the `YOUR_LINT_NAME_HERE` lint deny-by-default
- [ ] PR ? makes the `YOUR_LINT_NAE_HERE` lint a hard error

Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice to include:

Schedule

Describe the estimated schedule in terms of Rust releases for converting the warning to deny and then ultimately to a hard error.

Copy link
Member Author

Choose a reason for hiding this comment

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

Has any summary issue for a future-incompatibility lint included such a schedule?

I don't mind preserving parts of the original RFC that were good ideas, nor do I mind codifying existing practice, but in my quick skim earlier, I don't recall any future-incompatibility specifying a schedule.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not to my knowledge, but I think it would be good to have schedules for the purpose of effective tracking and triage; standardization of such schedules as Niko notes below would also be helpful imo.

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay; I don't dispute the idea that we need to have a more upfront focus on the scheduling. If @nikomatsakis signs off on the text you drafted, then I'll add it to this PR.

But if its something that's going to want a lot of discussion/tweaking/bikeshedding, then I'd rather leave it to a different Pull Request.

---------------------------------------------------------------------------

Expand Down Expand Up @@ -278,6 +282,10 @@ policy and not making any sort of breaking change at all:

N/A

# Amendments

* Amended template to match current practice.

<!-- -Links--------------------------------------------------------------------- -->

[RFC 1122]: https://github.com/rust-lang/rfcs/blob/master/text/1122-language-semver.md
Expand Down