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

Fix tests including dead_code warnings #10768

Closed
wants to merge 1 commit into from

Conversation

TaKO8Ki
Copy link
Member

@TaKO8Ki TaKO8Ki commented Jun 18, 2022

This patch is related to rust-lang/rust#97853. In it, I made some changes to collapse multiple dead code warnings into a single diagnostic. They broke some tests in cargo. After rust-lang/rust#97853 is merged, this PR should be merged.

@rust-highfive
Copy link

r? @ehuss

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 18, 2022
@TaKO8Ki TaKO8Ki force-pushed the fix-dead-code-warnings branch 2 times, most recently from 97c8c18 to c92a15b Compare June 19, 2022 09:39
@TaKO8Ki
Copy link
Member Author

TaKO8Ki commented Jun 22, 2022

rust-lang/rust#97853 was merged. After a nightly including it is released, this PR should be merged.

@ehuss
Copy link
Contributor

ehuss commented Jun 23, 2022

Thanks! As mentioned in rust-lang/rust#97853 (comment), when there are diagnostic changes that interfere with cargo's testsuite, the preferred route is to change the test to work with either version of rustc, merge that to master, and then update the submodule.

For example, these tests could have been modified to something like [WARNING] [..]`dead`[..] which would work with either version.

I'll go ahead and merge this to fix CI. If these kinds of changes come up in the future, we can further adjust these tests to be more general. Cargo doesn't really care about the specific error/warning text, just that something appears.

@bors r+

@bors
Copy link
Collaborator

bors commented Jun 23, 2022

📌 Commit 03a8490 has been approved by ehuss

@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 Jun 23, 2022
@bors
Copy link
Collaborator

bors commented Jun 23, 2022

⌛ Testing commit 03a8490 with merge 9e2062085df7582da853afaa0ffa050710ac60fb...

@bors
Copy link
Collaborator

bors commented Jun 23, 2022

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 23, 2022
@ehuss
Copy link
Contributor

ehuss commented Jun 23, 2022

I posted #10785 with the fixes to work on both stable and nightly. I'm going to go ahead and close this in favor of that.

@ehuss ehuss closed this Jun 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants