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

flake8 script file filter properties misbehave #202

Closed
arcivanov opened this Issue Sep 20, 2015 · 5 comments

Comments

Projects
None yet
2 participants
@arcivanov
Contributor

arcivanov commented Sep 20, 2015

Tried excluding bash files without .sh extensions from testing for flake8 by appending to a property, but was unsuccessful, had to disable flake8 on scripts entirely.

Looks like a bug, need to investigate.

@arcivanov arcivanov added the bug label Sep 20, 2015

@mriehl

This comment has been minimized.

Show comment
Hide comment
@mriehl

mriehl Sep 20, 2015

Member

Yes the flake8 plugin is currently not very clever when it comes to scripts linting. The problem is that you can't trust file extensions (scripts usually have a shebang and not an extension). So it would be necessary to analyze the semantics of the contents (which is hard) or the user would have to add or remove file names (which is annoying).

Maybe a regexp would be okay (default to .* but you could set it to "not ending with .sh") or maybe even a lambda / filter function.

Member

mriehl commented Sep 20, 2015

Yes the flake8 plugin is currently not very clever when it comes to scripts linting. The problem is that you can't trust file extensions (scripts usually have a shebang and not an extension). So it would be necessary to analyze the semantics of the contents (which is hard) or the user would have to add or remove file names (which is annoying).

Maybe a regexp would be okay (default to .* but you could set it to "not ending with .sh") or maybe even a lambda / filter function.

@arcivanov

This comment has been minimized.

Show comment
Hide comment
@arcivanov

arcivanov Sep 20, 2015

Contributor

The problem seems simpler than that: I appended ",hcsos*" to the exclusion list and it didn't exclude, i.e. there is something wrong with how we manipulate exclusions.

Contributor

arcivanov commented Sep 20, 2015

The problem seems simpler than that: I appended ",hcsos*" to the exclusion list and it didn't exclude, i.e. there is something wrong with how we manipulate exclusions.

@arcivanov

This comment has been minimized.

Show comment
Hide comment
@arcivanov

arcivanov Sep 20, 2015

Contributor

Yeah, well, that's not Flake8, it's PyB outsmarting itself...

Flake8 can process either directories or files. When directories are passed, you can include and exclude lists of patterns that Flake8 will follow. When specific files are passed patterns are disregarded by Flake8 (which makes complete sense).

@mriehl should we continue passing files to Flake8 and handle the patterns ourselves or should we just pass source directories along with uninterpreted patterns?

Example of behavior:

(current)bash-3.2$ flake8 -v src/main/scripts/hcsos_be_stop 
checking src/main/scripts/hcsos_be_stop
src/main/scripts/hcsos_be_stop:3:11: E225 missing whitespace around operator
src/main/scripts/hcsos_be_stop:3:25: E901 SyntaxError: invalid syntax
src/main/scripts/hcsos_be_stop:4:11: E225 missing whitespace around operator
(current)bash-3.2$ flake8 -v src/main/scripts/
directory src/main/scripts
checking src/main/scripts/_hcsos_be_server_start.py
checking src/main/scripts/_hcsos_populate_db.py
src/main/scripts/_hcsos_be_server_start.py:20:80: E501 line too long (89 > 79 characters)
src/main/scripts/_hcsos_be_server_start.py:52:80: E501 line too long (83 > 79 characters)
(current)bash-3.2$ 
Contributor

arcivanov commented Sep 20, 2015

Yeah, well, that's not Flake8, it's PyB outsmarting itself...

Flake8 can process either directories or files. When directories are passed, you can include and exclude lists of patterns that Flake8 will follow. When specific files are passed patterns are disregarded by Flake8 (which makes complete sense).

@mriehl should we continue passing files to Flake8 and handle the patterns ourselves or should we just pass source directories along with uninterpreted patterns?

Example of behavior:

(current)bash-3.2$ flake8 -v src/main/scripts/hcsos_be_stop 
checking src/main/scripts/hcsos_be_stop
src/main/scripts/hcsos_be_stop:3:11: E225 missing whitespace around operator
src/main/scripts/hcsos_be_stop:3:25: E901 SyntaxError: invalid syntax
src/main/scripts/hcsos_be_stop:4:11: E225 missing whitespace around operator
(current)bash-3.2$ flake8 -v src/main/scripts/
directory src/main/scripts
checking src/main/scripts/_hcsos_be_server_start.py
checking src/main/scripts/_hcsos_populate_db.py
src/main/scripts/_hcsos_be_server_start.py:20:80: E501 line too long (89 > 79 characters)
src/main/scripts/_hcsos_be_server_start.py:52:80: E501 line too long (83 > 79 characters)
(current)bash-3.2$ 
@arcivanov

This comment has been minimized.

Show comment
Hide comment
@arcivanov

arcivanov Sep 20, 2015

Contributor

For now I'll submit a PR to fix this by delegating filtering responsibility fully to flake8 and introducing "include" pattern into options.

Contributor

arcivanov commented Sep 20, 2015

For now I'll submit a PR to fix this by delegating filtering responsibility fully to flake8 and introducing "include" pattern into options.

@mriehl

This comment has been minimized.

Show comment
Hide comment
@mriehl

mriehl Sep 20, 2015

Member

👍 for letting flake8 decide

Member

mriehl commented Sep 20, 2015

👍 for letting flake8 decide

arcivanov added a commit to arcivanov/pybuilder that referenced this issue Sep 20, 2015

arcivanov added a commit to arcivanov/pybuilder that referenced this issue Sep 21, 2015

@arcivanov arcivanov modified the milestone: v0.11.2 Sep 23, 2015

@mriehl mriehl closed this in #204 Sep 23, 2015

@mriehl mriehl removed the in progress label Sep 23, 2015

mriehl added a commit that referenced this issue Sep 23, 2015

Merge pull request #204 from arcivanov/issue_202
Flake8 script file filter properties misbehave #202
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment