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

downgrade internal_features to warn #114692

Merged
merged 1 commit into from
Aug 12, 2023
Merged

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented Aug 10, 2023

Not sure if this requires an FCP or whatever. By having the lint as deny I need to modify test cases when testing them outside of the test suite as the test suite implicitly allows the lint. This takes maybe 10 to 20 seconds per test, but given just how frequently I end up copying tests to different repos it's a significant annoyance.

r? @Nilstrieb

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 10, 2023
@matthiaskrgr
Copy link
Member

matthiaskrgr commented Aug 10, 2023

edit: nevermind, misread the first comment :D

Maybe we can pass warn(internal_feature) to all tests globally (somewhere in compiletest config or something like that) instead of downgrading the lint?

@lcnr
Copy link
Contributor Author

lcnr commented Aug 10, 2023

My workflow is as follows: I copy tests into a separate repo and use rustc +rustX src/main.rs -Ztrait-solver=next. I use a separate repo because I want to generate mir-dumps and logs without adding garbage to my compiler directory which I then commit on accident.

I don't exclusively use cargo so I cannot simply change the lint via workspace configs.

@workingjubilee
Copy link
Member

per @Nilstrieb elsewhere:

[1:28 AM] nils:
I'm not logged in so I can't do anything on GitHub

when choosing the level I did think about the potential impact on testing stuff
I set it to deny in the hopes that it won't be too annoying. But if it is annoying, do downgrade it. I practice there really isn't that big of a difference between warn and deny because most serious rust projects are warning free so yeah, deny should probably only be used for things that are actual bugs, and using internal features isn't

[1:52 AM] nils: yeah, I don't think this needs any FCP or anything either, just r+

@bors r=Nilstrieb

@bors
Copy link
Contributor

bors commented Aug 12, 2023

📌 Commit 6888466 has been approved by Nilstrieb

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 12, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 12, 2023
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#94455 (Partially stabilize `int_roundings`)
 - rust-lang#114132 (Better Debug for Vars and VarsOs)
 - rust-lang#114584 (E0277 nolonger points at phantom `.await`)
 - rust-lang#114667 (Record binder for bare trait object in LifetimeCollectVisitor)
 - rust-lang#114692 (downgrade `internal_features` to warn)
 - rust-lang#114703 (Cover ParamConst in smir)
 - rust-lang#114734 (Mark oli as "on vacation")

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 70cd8d5 into rust-lang:master Aug 12, 2023
@rustbot rustbot added this to the 1.73.0 milestone Aug 12, 2023
@lcnr lcnr deleted the internal_features-warn branch August 14, 2023 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants