-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
A couple minor improvements for tidy error handling. #40653
Conversation
src/tools/tidy/src/bins.rs
Outdated
@@ -62,7 +62,7 @@ pub fn check(path: &Path, bad: &mut bool) { | |||
}); | |||
let path_bytes = rel_path.as_os_str().as_bytes(); | |||
if output.status.success() && output.stdout.starts_with(path_bytes) { | |||
println!("binary checked into source: {}", file.display()); | |||
tidy_error!("binary checked into source: {}", file.display()); | |||
*bad = true; |
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.
*bad = true
should probably a part of tidy_error
r=me with @petrochenkov's comment |
Going to wait for #40694 to merge. |
☔ The latest upstream changes (presumably #40950) made this pull request unmergeable. Please resolve the merge conflicts. |
Otherwise we get the standard Rust panic message alongside "some tidy checks failed" which seems unnecessary.
Comments have been addressed. |
@bors r=alexcrichton |
📌 Commit f789d89 has been approved by |
⌛ Testing commit f789d89 with merge 6719895... |
💔 Test failed - status-appveyor |
A couple minor improvements for tidy error handling. None
☀️ Test successful - status-appveyor, status-travis |
No description provided.