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

Split up transmute ui test #5068

Merged
merged 2 commits into from
Jan 21, 2020
Merged

Conversation

JohnTitor
Copy link
Member

Part of #2038

changelog: none

@flip1995
Copy link
Member

Is it now possible to add run-rustfix to the new test files?

@flip1995
Copy link
Member

$ wc -l tests/ui/*.stderr | sort -nr | head -n 7
 20404 total
   245 tests/ui/needless_range_loop.stderr
   245 tests/ui/match_same_arms.stderr
   235 tests/ui/non_copy_const.stderr
   234 tests/ui/matches.stderr
   220 tests/ui/drop_forget_ref.stderr
   213 tests/ui/indexing_slicing.stderr
   184 tests/ui/doc.stderr

Only 7 stderr files have more than 200 lines (hard limit target) now.

You can also reduce the limit to 245 now:

const LIMIT: usize = 275;

@JohnTitor
Copy link
Member Author

Is it now possible to add run-rustfix to the new test files?

We can't do at this point because applicability is unspecified in transmute_ptr_to_ptr/transmute_ptr_to_ref. But maybe we can do it as follow-up work.

You can also reduce the limit to 245 now

I think the 200 line limit is in the near future. But if it is appropriate to reduce the limit little by little, I'll do so.

@flip1995
Copy link
Member

We can't do at this point because applicability is unspecified

Ah I remember now. Back when I added Applicability to every lint, I was to lazy to read all the transmute code to figure out the applicability of these lints 😄

So I guess follow up it is 👍

But if it is appropriate to reduce the limit little by little, I'll do so.

I think we should do this, so that we won't get new bigger test files again. Also @phansch did this now and then in some of these kind of PRs.

@JohnTitor
Copy link
Member Author

I think we should do this, so that we won't get new bigger test files again.

It's reasonable. Decreased the line length limit.

@phansch
Copy link
Member

phansch commented Jan 21, 2020

@bors r+ thanks!

@bors
Copy link
Contributor

bors commented Jan 21, 2020

📌 Commit c9d5cb9 has been approved by phansch

@bors
Copy link
Contributor

bors commented Jan 21, 2020

⌛ Testing commit c9d5cb9 with merge f78cc07...

bors added a commit that referenced this pull request Jan 21, 2020
Split up `transmute` ui test

Part of #2038

changelog: none
@bors
Copy link
Contributor

bors commented Jan 21, 2020

☀️ Test successful - checks-travis, status-appveyor
Approved by: phansch
Pushing f78cc07 to master...

@bors bors merged commit c9d5cb9 into rust-lang:master Jan 21, 2020
@JohnTitor JohnTitor deleted the split-up-transmute branch January 21, 2020 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants