fix: 🐛 do not exclude files in explicitly included directorys#850
fix: 🐛 do not exclude files in explicitly included directorys#850Katze719 wants to merge 4 commits intopython-poetry:mainfrom
Conversation
Reviewer's Guide by SourceryThis pull request introduces a new method, Updated class diagram for BuilderclassDiagram
class Builder {
-_path: Path
-_poetry: Poetry
-_env: Env
-_excluded_files: set[str] | None
-_package: Package
+build(target_dir: Path | None) : Path
+find_excluded_files(fmt: str | None = None) : set[str]
-_apply_local_version_label() : None
-_get_ignored_files(vcs_ignored_files, explicitly_excluded, explicitly_included) : set[str]
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey @Katze719 - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a unit test for the
_get_ignored_filesmethod to ensure it behaves as expected with different combinations of vcs ignored files, explicitly excluded files, and explicitly included files.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| def _get_ignored_files(self, vcs_ignored_files, explicitly_excluded, explicitly_included): | ||
| """ | ||
| Returns the set of ignored files based on: | ||
| - vcs_ignored_files: files or directories ignored by the version control system, | ||
| - explicitly_excluded: files explicitly marked for exclusion, | ||
| - explicitly_included: files or directories explicitly marked for inclusion. | ||
|
|
||
| If a directory is explicitly included, all files and subdirectories within it | ||
| will not be ignored even if they appear in vcs_ignored_files. | ||
| """ |
There was a problem hiding this comment.
question: Review path resolution consistency when comparing paths.
The method resolves each candidate and the explicitly included paths using Path.resolve(strict=False). Please ensure that all these paths are in a consistent format (e.g., all absolute) and relative context is correctly accounted for, especially since explicitly included files are added relative to self._path in find_excluded_files. This consistency is critical to avoid mismatches in path comparisons.
|
NVM using poetry_test_issue_10259/myLibs/**/* works 🙃🙃🙃 do what ever you want with this code |
|
maybe, could we add to the documentation that globing is supported for including/excluding files within the pyproject.toml? |
Resolves: python-poetry#10259
Summary by Sourcery
Bug Fixes: