-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Include configured src directories when resolving graphs
#22451
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
Conversation
src directories when resolving graphs
|
| pyproject_config | ||
| .settings | ||
| .linter | ||
| .src | ||
| .iter() | ||
| .filter(|path| path.is_dir()) | ||
| .filter_map(|path| SystemPathBuf::from_path_buf(path.clone()).ok()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This field supports globs. For example, if you have a series of Python packages in a python_modules directory, src = ["python_modules/*"] would expand to incorporate all packages in that directory. User home directory and environment variables will also be expanded.
Are globs and environment variables already expanded at this point?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I believe so, I think they're expanded by the time they reach LinterSettings.
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
|
MichaReiser
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add a test using a glob for src to confirm that they're expanded at this point.
| for path in &pyproject_config.settings.linter.src { | ||
| if path.is_dir() { | ||
| if let Ok(path) = SystemPathBuf::from_path_buf(path.clone()) { | ||
| src_roots.insert(path); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Add detected package roots. | ||
| for path in package_roots | ||
| .values() | ||
| .filter_map(|package| package.as_deref()) | ||
| .filter_map(|package| package.parent()) | ||
| .map(Path::to_path_buf) | ||
| .filter_map(|path| SystemPathBuf::from_path_buf(path).ok()) | ||
| .collect(); | ||
| { | ||
| if let Some(parent) = path.parent() { | ||
| if let Ok(path) = SystemPathBuf::from_path_buf(parent.to_path_buf()) { | ||
| src_roots.insert(path); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit:
src_roots.extend(
pyproject_config.settings.linter.src.iter()
.filter(|path| path.is_dir())
.filter_map(|path| SystemPathBuf::from_path_buf(path.clone());
...|
🫡 |
d19d3a2 to
575dea9
Compare
Summary
This PR augments the detected source paths with the user-configured
srcwhen computing roots forruff analyze graph.