Skip to content

Commit dda4d7b

Browse files
committed
slightly correct comments and diagnostics about checking modifications
I feel like they are still wrong, but maybe less so .-. The `info:` was unhelpful -- we only use upstream in CI nowdays.
1 parent 52f4b16 commit dda4d7b

File tree

2 files changed

+13
-10
lines changed

2 files changed

+13
-10
lines changed

Diff for: src/bootstrap/src/core/build_steps/format.rs

+9-8
Original file line numberDiff line numberDiff line change
@@ -85,11 +85,15 @@ fn update_rustfmt_version(build: &Builder<'_>) {
8585
t!(stamp_file.add_stamp(version).write());
8686
}
8787

88-
/// Returns the Rust files modified between the `merge-base` of HEAD and
89-
/// rust-lang/master and what is now on the disk. Does not include removed files.
88+
/// Returns the Rust files modified between the last merge commit and what is now on the disk.
89+
/// Does not include removed files.
9090
///
9191
/// Returns `None` if all files should be formatted.
9292
fn get_modified_rs_files(build: &Builder<'_>) -> Result<Option<Vec<String>>, String> {
93+
// In CI `get_git_modified_files` returns something different to normal environment.
94+
// This shouldn't be called in CI anyway.
95+
assert!(!build.config.is_running_on_ci);
96+
9397
if !verify_rustfmt_version(build) {
9498
return Ok(None);
9599
}
@@ -104,7 +108,7 @@ struct RustfmtConfig {
104108

105109
// Prints output describing a collection of paths, with lines such as "formatted modified file
106110
// foo/bar/baz" or "skipped 20 untracked files".
107-
fn print_paths(build: &Builder<'_>, verb: &str, adjective: Option<&str>, paths: &[String]) {
111+
fn print_paths(verb: &str, adjective: Option<&str>, paths: &[String]) {
108112
let len = paths.len();
109113
let adjective =
110114
if let Some(adjective) = adjective { format!("{adjective} ") } else { String::new() };
@@ -115,9 +119,6 @@ fn print_paths(build: &Builder<'_>, verb: &str, adjective: Option<&str>, paths:
115119
} else {
116120
println!("fmt: {verb} {len} {adjective}files");
117121
}
118-
if len > 1000 && !build.config.is_running_on_ci {
119-
println!("hint: if this number seems too high, try running `git fetch origin master`");
120-
}
121122
}
122123

123124
pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
@@ -190,7 +191,7 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
190191
)
191192
.map(|x| x.to_string())
192193
.collect();
193-
print_paths(build, "skipped", Some("untracked"), &untracked_paths);
194+
print_paths("skipped", Some("untracked"), &untracked_paths);
194195

195196
for untracked_path in untracked_paths {
196197
// The leading `/` makes it an exact match against the
@@ -319,7 +320,7 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
319320
});
320321
let mut paths = formatted_paths.into_inner().unwrap();
321322
paths.sort();
322-
print_paths(build, if check { "checked" } else { "formatted" }, adjective, &paths);
323+
print_paths(if check { "checked" } else { "formatted" }, adjective, &paths);
323324

324325
drop(tx);
325326

Diff for: src/build_helper/src/git.rs

+4-2
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,9 @@ fn git_upstream_merge_base(
114114
Ok(output_result(git.arg("merge-base").arg(&updated_master).arg("HEAD"))?.trim().to_owned())
115115
}
116116

117-
/// Searches for the nearest merge commit in the repository that also exists upstream.
117+
/// Searches for the nearest merge commit in the repository.
118+
///
119+
/// **In CI** finds the nearest merge commit that *also exists upstream*.
118120
///
119121
/// It looks for the most recent commit made by the merge bot by matching the author's email
120122
/// address with the merge bot's email.
@@ -165,7 +167,7 @@ pub fn get_closest_merge_commit(
165167
Ok(output_result(&mut git)?.trim().to_owned())
166168
}
167169

168-
/// Returns the files that have been modified in the current branch compared to the master branch.
170+
/// Returns the files that have been modified in the current branch compared to the last merge.
169171
/// The `extensions` parameter can be used to filter the files by their extension.
170172
/// Does not include removed files.
171173
/// If `extensions` is empty, all files will be returned.

0 commit comments

Comments
 (0)