Stop at the first resolved parent configuration#8864
Merged
MichaReiser merged 3 commits intomainfrom Nov 29, 2023
Merged
Conversation
Member
Author
|
Current dependencies on/for this PR: This stack of pull requests is managed by Graphite. |
Member
Author
|
This is a potential breaking change |
Contributor
|
charliermarsh
approved these changes
Nov 28, 2023
Member
charliermarsh
left a comment
There was a problem hiding this comment.
This looks correct to me.
c694d4c to
c8a7db5
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Summary
This is a potential fix for #8858 but requires some more testing.
I'm throwing this out here to get some feedback on whether this seems correct. The following is my reasoning, although I'm not very familiar with the code.
The idea of visiting all ancestor paths is to find a configuration for the current directory (or given path).
Visiting the ancestors is necessary if the configuration happens to be in a parent directory. For example when running ruff in a sub-directory of the project.
This PR changes the implementation to stop at the first found configuration instead of visiting the entire project chain.
One potential issue this could introduce is:
Running ruff in
sub-sub-dirwould lint no files in the old version because the files are excluded by the root pyproject.toml.The new version lints the files in the sub-sub-dir because it finds the
subdir/pyproject.tomlas the closest pyproject.toml and itdoes not exclude the directoy.
The new behavior seems more correct to me, but I might be overlooking something.
Fixes #8858
Test Plan
Added regression test.