Skip to content

Walk proto dirs instead of manually specifying files #19

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

Merged
merged 1 commit into from
Apr 4, 2025

Conversation

graysonguarino
Copy link
Collaborator

Issue Number:
Reference the issue this PR fixes.

Description of Changes:
Provide a clear and concise explanation of what changes you made and why.

  • Walk protobuf directory instead of needing to manually specify which files are to be included.

Testing Done:
How did you test your changes? Share details like steps, tools used, or results.

Terms of contribution:

By submitting this pull request, I agree that this contribution is licensed under the terms of the Apache License, Version 2.0.


Thanks for submitting your pull request! We will review it as soon as possible.

@graysonguarino graysonguarino requested a review from ecpullen April 4, 2025 14:22
@graysonguarino graysonguarino force-pushed the server/walk-proto-dirs branch from 443f571 to 6663fd3 Compare April 4, 2025 14:25
Comment on lines 31 to 55
fn fetch_proto_dirs(
proto_package_dir: &Path,
proto_version: usize,
ignore_modules: Vec<&str>,
) -> std::io::Result<Vec<PathBuf>> {
let version_str = format!("v{proto_version}");

let ignore_components = ignore_modules
.iter()
.map(|module| Component::Normal(OsStr::new(module)))
.collect::<Vec<_>>();

let proto_paths = WalkDir::new(proto_package_dir)
.into_iter()
.filter(|entry| {
let entry = if let Ok(entry) = entry {
entry
} else {
return false;
};

entry.path().extension().is_some_and(|ext| ext == "proto")
&& entry
.path()
.components()
.any(|comp| comp == Component::Normal(OsStr::new(&version_str)))
&& !entry
.path()
.components()
.any(|comp| ignore_components.contains(&comp))
})
.collect::<Result<Vec<_>, _>>()?;

Ok(proto_paths
.iter()
.map(|entry| entry.path().to_path_buf())
.collect::<Vec<PathBuf>>())
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this any cleaner?

Suggested change
fn fetch_proto_dirs(
proto_package_dir: &Path,
proto_version: usize,
ignore_modules: Vec<&str>,
) -> std::io::Result<Vec<PathBuf>> {
let version_str = format!("v{proto_version}");
let ignore_components = ignore_modules
.iter()
.map(|module| Component::Normal(OsStr::new(module)))
.collect::<Vec<_>>();
let proto_paths = WalkDir::new(proto_package_dir)
.into_iter()
.filter(|entry| {
let entry = if let Ok(entry) = entry {
entry
} else {
return false;
};
entry.path().extension().is_some_and(|ext| ext == "proto")
&& entry
.path()
.components()
.any(|comp| comp == Component::Normal(OsStr::new(&version_str)))
&& !entry
.path()
.components()
.any(|comp| ignore_components.contains(&comp))
})
.collect::<Result<Vec<_>, _>>()?;
Ok(proto_paths
.iter()
.map(|entry| entry.path().to_path_buf())
.collect::<Vec<PathBuf>>())
}
fn fetch_proto_dirs(
proto_package_dir: &Path,
proto_version: usize,
ignore_modules: &[&str],
) -> std::io::Result<Vec<PathBuf>> {
let version_str = format!("v{proto_version}");
Ok(WalkDir::new(proto_package_dir)
.into_iter()
// Remove all files that don't have read permission
.filter_map(|e| e.ok())
.filter(|entry| entry.path().extension().is_some_and(|ext| ext == "proto"))
.filter(|entry| entry.path().components().filter_map(|comp| comp.as_os_str().to_str()).any(|comp| comp == version_str))
.filter(|entry| !entry.path().components().filter_map(|comp| comp.as_os_str().to_str()).any(|comp| ignore_modules.contains(&comp)))
.map(|entry| entry.path().to_path_buf()).collect())
}

@graysonguarino graysonguarino force-pushed the server/walk-proto-dirs branch from 6663fd3 to 8675ce9 Compare April 4, 2025 16:40
@graysonguarino graysonguarino merged commit 6b305e9 into mvp Apr 4, 2025
1 check passed
@graysonguarino graysonguarino deleted the server/walk-proto-dirs branch April 4, 2025 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants