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

Amend the template from RFC 1589 #2658

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

Conversation

pnkfelix
Copy link
Member

@pnkfelix pnkfelix commented Mar 8, 2019

Amend the issue template from text of RFC 1589 to match current actual practice of future-compatibility lints.

In particular, while working on rust-lang/rust#58608, I discovered that the current batch of C-future-compatibility tracking issues don't look anything like the template provided in this RFC.

So I took inspiration from one them (rust-lang/#57644 in particular) when composing my own rust-lang/rust#59014, but I wanted to back-propagate the template that seems to have evolved into the original RFC text so that people who use the RFC as the basis for their own issue will start off on the right foot.

text/1589-rustc-bug-fix-procedure.md Show resolved Hide resolved

- [ ] 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.

@Centril Centril requested a review from nikomatsakis March 8, 2019 16:12
@Centril
Copy link
Contributor

Centril commented Mar 8, 2019

The https://forge.rust-lang.org/rustc-bug-fix-procedure.html page also needs updating after this. :)


*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?

text/1589-rustc-bug-fix-procedure.md Show resolved Hide resolved
…late.

Since that initial paragraph includes the link to RFC 1589, I removed
the suggestion (that I added) to include the link from the "What is
this lint about" section.
@pnkfelix pnkfelix requested a review from nikomatsakis March 13, 2019 12:35
@Centril
Copy link
Contributor

Centril commented Mar 13, 2019

I think the current state of this PR is good and we should merge this for now. Follow up work is discussed in https://rust-lang.zulipchat.com/#narrow/stream/185694-t-compiler.2Fwg-meta/topic/future-incompat.20planning.

@Centril Centril added A-lint Proposals relating to lints / warnings / clippy. A-governance Proposals relating to how Rust is governed. labels Mar 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-governance Proposals relating to how Rust is governed. A-lint Proposals relating to lints / warnings / clippy.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants