Skip to content

Commit 35ae73a

Browse files
authored
Removing unused for-file code (#67)
* removing unused for-file code * show version * adding the version * avoiding panics
1 parent ae6332e commit 35ae73a

File tree

9 files changed

+75
-291
lines changed

9 files changed

+75
-291
lines changed

dev/run_benchmarks_for_file.sh

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,5 +11,4 @@ echo "To run these benchmarks on your application, you can place this repo next
1111

1212
hyperfine --warmup=2 --runs=3 --export-markdown tmp/codeowners_for_file_benchmarks.md \
1313
"../rubyatscale/codeowners-rs/target/release/codeowners for-file \"$1\"" \
14-
"bin/codeowners for_file \"$1\"" \
1514
"bin/codeownership for_file \"$1\""

dev/run_benchmarks_for_gv.sh

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,4 @@ echo "To run these benchmarks on your application, you can place this repo next
99

1010
hyperfine --warmup=2 --runs=3 --export-markdown tmp/codeowners_benchmarks_gv.md \
1111
'../rubyatscale/codeowners-rs/target/release/codeowners gv' \
12-
'bin/codeowners validate' \
1312
'bin/codeownership validate'

src/bin/compare_for_file.rs

Lines changed: 0 additions & 179 deletions
This file was deleted.

src/cli.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use path_clean::PathClean;
66
use std::path::{Path, PathBuf};
77

88
#[derive(Subcommand, Debug)]
9+
#[command(version)]
910
enum Command {
1011
#[clap(about = "Finds the owner of a given file.", visible_alias = "f")]
1112
ForFile {
@@ -109,7 +110,7 @@ pub fn cli() -> Result<RunResult, RunnerError> {
109110
Command::Validate => runner::validate(&run_config, vec![]),
110111
Command::Generate { skip_stage } => runner::generate(&run_config, !skip_stage),
111112
Command::GenerateAndValidate { skip_stage } => runner::generate_and_validate(&run_config, vec![], !skip_stage),
112-
Command::ForFile { name, fast } => runner::for_file(&run_config, &name, fast),
113+
Command::ForFile { name, fast: _ } => runner::for_file(&run_config, &name),
113114
Command::ForTeam { name } => runner::for_team(&run_config, &name),
114115
Command::DeleteCache => runner::delete_cache(&run_config),
115116
};

src/ownership/parser.rs

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -59,17 +59,23 @@ impl Parser {
5959
fn teams_by_github_team_name(team_file_glob: Vec<String>) -> HashMap<String, Team> {
6060
let mut teams = HashMap::new();
6161
for glob in team_file_glob {
62-
let paths = glob::glob(&glob).expect("Failed to read glob pattern").filter_map(Result::ok);
63-
64-
for path in paths {
65-
let team = match Team::from_team_file_path(path) {
66-
Ok(team) => team,
67-
Err(e) => {
68-
eprintln!("Error parsing team file: {}", e);
69-
continue;
62+
match glob::glob(&glob) {
63+
Ok(paths) => {
64+
for path in paths.filter_map(Result::ok) {
65+
let team = match Team::from_team_file_path(path) {
66+
Ok(team) => team,
67+
Err(e) => {
68+
eprintln!("Error parsing team file: {}", e);
69+
continue;
70+
}
71+
};
72+
teams.insert(team.github_team.clone(), team);
7073
}
71-
};
72-
teams.insert(team.github_team.clone(), team);
74+
}
75+
Err(e) => {
76+
eprintln!("Failed to read glob pattern '{}': {}", glob, e);
77+
continue;
78+
}
7379
}
7480
}
7581

src/project_builder.rs

Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,25 @@ impl<'a> ProjectBuilder<'a> {
7171
})
7272
});
7373

74-
// Process sequentially with &mut self
75-
let collected_entries = Arc::try_unwrap(collected).unwrap().into_inner().unwrap();
74+
// Process sequentially with &mut self without panicking on Arc/Mutex unwraps
75+
let collected_entries = match Arc::try_unwrap(collected) {
76+
// We are the sole owner of the Arc
77+
Ok(mutex) => match mutex.into_inner() {
78+
// Mutex not poisoned
79+
Ok(entries) => entries,
80+
// Recover entries even if the mutex was poisoned
81+
Err(poisoned) => poisoned.into_inner(),
82+
},
83+
// There are still other Arc references; lock and take the contents
84+
Err(arc) => match arc.lock() {
85+
Ok(mut guard) => std::mem::take(&mut *guard),
86+
// Recover guard even if poisoned, then take contents
87+
Err(poisoned) => {
88+
let mut guard = poisoned.into_inner();
89+
std::mem::take(&mut *guard)
90+
}
91+
},
92+
};
7693
for entry in collected_entries {
7794
entry_types.push(self.build_entry_type(entry)?);
7895
}
@@ -89,11 +106,10 @@ impl<'a> ProjectBuilder<'a> {
89106
if is_dir {
90107
return Ok(EntryType::Directory(absolute_path.to_owned(), relative_path.to_owned()));
91108
}
92-
let file_name = relative_path
93-
.file_name()
94-
.expect("expected a file_name")
95-
.to_string_lossy()
96-
.to_lowercase();
109+
let file_name = match relative_path.file_name() {
110+
Some(name) => name.to_string_lossy().to_lowercase(),
111+
None => return Ok(EntryType::NullEntry()),
112+
};
97113

98114
match file_name.as_str() {
99115
name if name == "package.yml"
@@ -142,11 +158,14 @@ impl<'a> ProjectBuilder<'a> {
142158
}
143159
EntryType::Directory(absolute_path, relative_path) => {
144160
if relative_path.parent() == Some(Path::new(&self.config.vendored_gems_path)) {
145-
let file_name = relative_path.file_name().expect("expected a file_name");
146-
gems.push(VendoredGem {
147-
path: absolute_path,
148-
name: file_name.to_string_lossy().to_string(),
149-
});
161+
if let Some(file_name) = relative_path.file_name() {
162+
gems.push(VendoredGem {
163+
path: absolute_path,
164+
name: file_name.to_string_lossy().to_string(),
165+
});
166+
} else {
167+
warn!("Vendored gem path without file name: {:?}", relative_path);
168+
}
150169
}
151170
}
152171
EntryType::RubyPackage(absolute_path, relative_path) => {

0 commit comments

Comments
 (0)