-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Add a stderr file length check to clippy_dev #4100
Conversation
539fcf0
to
5431392
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. There are some empty lines (17, 55) in stderr_length_check.rs
. (rustfmt should have removed these? 🤔)
} else { | ||
None | ||
} | ||
}).collect() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not returning impl Iterator<Item = String>
, instead of collecting?
This adds a check to `clippy_dev` that enforces a maximum line count for `stderr` files. CI will fail if the line count is exceeded. It's currently set to `320` lines. Ideally this would be implemented in `compiletest-rs` but there are plans to move Rust's `compiletest` into the `compiletest-rs` repository and I don't want to do the work in `compiletest` twice. However, I also don't want to wait until the move is done, so I added the check to `clippy_dev` until it makes sense to add it to compiletest-rs. cc rust-lang#2038
63be8c6
to
d9a8a8a
Compare
Uhh, I've never seen this failure before and a job restart didn't fix it either: https://travis-ci.com/rust-lang/rust-clippy/jobs/200943066#L886
|
let stderr_files = stderr_files(); | ||
let exceeding_files = exceeding_stderr_files(stderr_files).collect::<Vec<String>>(); | ||
|
||
if !exceeding_files.is_empty() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh the collection is needed because of this. In this case it doesn't matter if we collect inside the function or here. Sorry for that :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's still better to collect at the call site rather than in the function itself, so it's fine 👍
r=me with travis green |
I couldn't reproduce the issue locally and will have another look later today. Why can't it find the .rmeta file for serde, and serde only? I couldn't find any recent changes in Clippy or Rust that look like they would cause this. Any pointers appreciated =) edit: opened #4108 because it also happens in other PRs |
@bors r=flip1995 |
📌 Commit 619a290 has been approved by |
Add a stderr file length check to clippy_dev This adds a check to `clippy_dev` that enforces a maximum line count for `stderr` files. CI will fail if the line count is exceeded. It's currently set to `320` lines. Ideally this would be implemented in `compiletest-rs` but there are plans to move Rust's `compiletest` into the `compiletest-rs` repository and I don't want to do the work in `compiletest` twice. However, I also don't want to wait until the move is done, so I added the check to `clippy_dev` until it makes sense to add it to compiletest-rs. cc #2038
☀️ Test successful - checks-travis, status-appveyor |
This adds a check to
clippy_dev
that enforces a maximum line count forstderr
files. CI will fail if the line count is exceeded. It'scurrently set to
320
lines.Ideally this would be implemented in
compiletest-rs
but there areplans to move Rust's
compiletest
into thecompiletest-rs
repositoryand I don't want to do the work in
compiletest
twice. However, I alsodon't want to wait until the move is done, so I added the check to
clippy_dev
until it makes sense to add it to compiletest-rs.changelog: none
cc #2038