Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 0ae5bae

Browse files
committedApr 1, 2024
optimize tidy check on src/tools/tidy/src/issues.txt
This change applies to the following: - Handles `is_sorted` in the first iteration without needing a second. - Fixes line sorting on `--bless`. - Reads `issues.txt` as str rather than making it part of the source code. Signed-off-by: onur-ozkan <[email protected]>
1 parent 3d5528c commit 0ae5bae

File tree

2 files changed

+4388
-4397
lines changed

2 files changed

+4388
-4397
lines changed
 

‎src/tools/tidy/src/issues.txt

Lines changed: 4372 additions & 4380 deletions
Large diffs are not rendered by default.

‎src/tools/tidy/src/ui_tests.rs

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -105,18 +105,27 @@ pub fn check(root_path: &Path, bless: bool, bad: &mut bool) {
105105

106106
// the list of files in ui tests that are allowed to start with `issue-XXXX`
107107
// BTreeSet because we would like a stable ordering so --bless works
108-
let issues_list =
109-
include!("issues.txt").map(|path| path.replace("/", std::path::MAIN_SEPARATOR_STR));
110-
let issues: Vec<String> = Vec::from(issues_list.clone());
111-
let is_sorted = issues.windows(2).all(|w| w[0] < w[1]);
108+
let mut prev_line = String::new();
109+
let mut is_sorted = true;
110+
let allowed_issue_names: BTreeSet<_> = include_str!("issues.txt")
111+
.lines()
112+
.map(|line| {
113+
if *prev_line > *line {
114+
is_sorted = false;
115+
}
116+
117+
prev_line = line.to_string();
118+
line.to_string()
119+
})
120+
.collect();
121+
112122
if !is_sorted && !bless {
113123
tidy_error!(
114124
bad,
115125
"`src/tools/tidy/src/issues.txt` is not in order, mostly because you modified it manually,
116126
please only update it with command `x test tidy --bless`"
117127
);
118128
}
119-
let allowed_issue_names = BTreeSet::from(issues_list);
120129

121130
let mut remaining_issue_names: BTreeSet<String> = allowed_issue_names.clone();
122131

@@ -186,29 +195,19 @@ pub fn check(root_path: &Path, bless: bool, bad: &mut bool) {
186195
// if there are any file names remaining, they were moved on the fs.
187196
// our data must remain up to date, so it must be removed from issues.txt
188197
// do this automatically on bless, otherwise issue a tidy error
189-
if bless && !remaining_issue_names.is_empty() {
190-
let issues_txt_header = r#"
191-
/*
192-
============================================================
193-
⚠️⚠️⚠️NOTHING SHOULD EVER BE ADDED TO THIS LIST⚠️⚠️⚠️
194-
============================================================
195-
*/
196-
[
197-
"#;
198+
if bless && (!remaining_issue_names.is_empty() || !is_sorted) {
198199
let tidy_src = root_path.join("src/tools/tidy/src");
199200
// instead of overwriting the file, recreate it and use an "atomic rename"
200201
// so we don't bork things on panic or a contributor using Ctrl+C
201202
let blessed_issues_path = tidy_src.join("issues_blessed.txt");
202203
let mut blessed_issues_txt = fs::File::create(&blessed_issues_path).unwrap();
203-
blessed_issues_txt.write(issues_txt_header.as_bytes()).unwrap();
204204
// If we changed paths to use the OS separator, reassert Unix chauvinism for blessing.
205205
for filename in allowed_issue_names
206206
.difference(&remaining_issue_names)
207207
.map(|s| s.replace(std::path::MAIN_SEPARATOR_STR, "/"))
208208
{
209-
write!(blessed_issues_txt, "\"{filename}\",\n").unwrap();
209+
writeln!(blessed_issues_txt, "{filename}").unwrap();
210210
}
211-
write!(blessed_issues_txt, "]\n").unwrap();
212211
let old_issues_path = tidy_src.join("issues.txt");
213212
fs::rename(blessed_issues_path, old_issues_path).unwrap();
214213
} else {

0 commit comments

Comments
 (0)
Please sign in to comment.