Quieter tidy #146316
Quieter tidy #146316
Conversation
rustbot
added
A-testsuite
|
I think the first two commits should be uncontroversial, because they are about individual tidy checks that are noisy. The third one I'm less sure about, because it's removing top-level tidy progress indicators. |
| eprintln!("Take a look at E0001 to see how to handle it."); | ||
| return; | ||
| } | ||
| println!("No error code explanation was removed!"); |
There was a problem hiding this comment.
These messages are useful, especially in regression checks later on if this code because outdated (ie, how error codes are handled changed).
There was a problem hiding this comment.
I really don't think we should print messages of the form "there was no error in XYZ". That's about as user-friendly as rustc printing
No type error found.
No lifetime error found.
No violated trait bound found.
...
There's a good reason we don't do that.
Testing-only messages useful for debugging the tidy itself not be enabled by default in production.
| // First we check that `src/rustdoc-json-types` was modified. | ||
| if !crate::files_modified(ci_info, |p| p == RUSTDOC_JSON_TYPES) { | ||
| // `rustdoc-json-types` was not modified so nothing more to check here. | ||
| println!("`rustdoc-json-types` was not modified."); |
|
I also agree that the messages that say that something was checked are quite useful, because tidy actually has a lot of optional functionality ( What I think we could improve though is the error message when something bad actually happens, because that is sometimes a bit drowned in the rest of the text. So the errors should BE LOUD AND VISIBLE, so that you can always quickly find out if an error happened or not. Maybe tidy also say something like "everything was fine" at the very end, ideally printed in green, to help us figure out if the result was ok or not. |
|
If you have a message per step, I think I'd slightly prefer to have "Checking x..." at the start of the step than have "x is good" at the end of the step. |
|
Well, I guess my intution about this is off. |
rustbot
removed
the
S-waiting-on-review
|
I submitted something similar at #146580. |
Implement a simple diagnostic system for tidy In #146316 and #146580, contributors independently wanted to reduce the verbose output of tidy. But before, the output was quite ad-hoc, so it was not easy to control it. In this PR, I implemented a simple diagnostic system for tidy, which allows us to: 1) Only print certain information in verbose mode (`-v`) 2) Associate each (error) output to a specific check, so that it is easier to find out what exactly has failed and which check you might want to examine (not fully done, there are some random `println`s left, but most output should be scoped to a specific check) 3) Print output with colors, based on the message level (message, warning, error) 4) Show the start/end execution of each check in verbose mode, for better progress indication Failure output: <img width="1134" height="157" alt="image" src="https://github.com/user-attachments/assets/578a9302-e1c2-47e5-9370-a3556c49d9fc" /> Success output: <img width="388" height="113" alt="image" src="https://github.com/user-attachments/assets/cf27faf8-3d8b-49e3-88d0-fac27a9c36a8" /> Verbose output (shortened): <img width="380" height="158" alt="image" src="https://github.com/user-attachments/assets/ce7102b8-c2f3-42a8-a2ec-ca30389be91e" /> CC `@nnethercote` `@RalfJung` `@GuillaumeGomez` The first two commits and the last commit are interesting, the rest is just mechanical port of the code from `bad: &mut bool` to `DiagCtx` and `RunningCheck`. The `extra_checks` check could be further split, but I'd leave that for another PR. r? `@jieyouxu`
Implement a simple diagnostic system for tidy In rust-lang#146316 and rust-lang#146580, contributors independently wanted to reduce the verbose output of tidy. But before, the output was quite ad-hoc, so it was not easy to control it. In this PR, I implemented a simple diagnostic system for tidy, which allows us to: 1) Only print certain information in verbose mode (`-v`) 2) Associate each (error) output to a specific check, so that it is easier to find out what exactly has failed and which check you might want to examine (not fully done, there are some random `println`s left, but most output should be scoped to a specific check) 3) Print output with colors, based on the message level (message, warning, error) 4) Show the start/end execution of each check in verbose mode, for better progress indication Failure output: <img width="1134" height="157" alt="image" src="https://github.com/user-attachments/assets/578a9302-e1c2-47e5-9370-a3556c49d9fc" /> Success output: <img width="388" height="113" alt="image" src="https://github.com/user-attachments/assets/cf27faf8-3d8b-49e3-88d0-fac27a9c36a8" /> Verbose output (shortened): <img width="380" height="158" alt="image" src="https://github.com/user-attachments/assets/ce7102b8-c2f3-42a8-a2ec-ca30389be91e" /> CC `@nnethercote` `@RalfJung` `@GuillaumeGomez` The first two commits and the last commit are interesting, the rest is just mechanical port of the code from `bad: &mut bool` to `DiagCtx` and `RunningCheck`. The `extra_checks` check could be further split, but I'd leave that for another PR. r? `@jieyouxu`
When I run
tidyI currently get output like this:Several of these output lines seem unnecessary. E.g. there are lots of different tidy tests, why do a couple of them print output saying that they don't need to do anything?
r? @GuillaumeGomez