Skip to content
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

Fix check_paths definition for pep8tool #300

Merged
merged 2 commits into from
Nov 29, 2018

Conversation

chocoelho
Copy link
Contributor

See #257 (comment), and check this fix against this repo.

Seems like the way check_paths was defined it excluded some paths before checking the full path of folders, leaving some of them out of the inspection depending on the given path.

Signed-off-by: Carlos Coelho <carlospecter@gmail.com>
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 77.688% when pulling bc5663d on vintasoftware:fix-pep8-filepath into 4cc89a3 on PyCQA:develop.

@coveralls
Copy link

coveralls commented Nov 23, 2018

Coverage Status

Coverage decreased (-0.01%) to 77.688% when pulling 685b24b on vintasoftware:fix-pep8-filepath into 264d0ad on PyCQA:develop.

@chocoelho
Copy link
Contributor Author

@carlio just to give some context on what's happening here, prior to this change:

path_a/
---- path_b/
-------- path_c/

If prospector is ran at the current folder (say path_a) prospector ., path_c won't be listed among found files/folders for pep8 tool so it won't throw any warnings/errors if any exists and this folder (path_c) will only show up if prospector is ran explicitly against path_a, e.g. prospector path_a. What I found is that the removed line would skip listing path_c, because it'd be considered as already inside check_pathsvariable (checked that by debugging).

@carlio
Copy link
Member

carlio commented Nov 29, 2018

Well I absolutely can't remember the reason for that initial iteration so sure :-)

@carlio carlio merged commit 5e1cb52 into landscapeio:develop Nov 29, 2018
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.

None yet

3 participants