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

Add linting tool: flake8 both to pyproject.toml and CI pipeline #21

Merged
merged 6 commits into from
Mar 21, 2024

Conversation

abzrg
Copy link
Contributor

@abzrg abzrg commented Mar 21, 2024

Hi,

With some investigations, I added an step in the lint job of github actions for flake8. Hope it's correct.

For now, I added all the errors and warnings emitted by flake8 to the ignore entry to keep these commits simple.
One of these warnings was about the length of the line. This can be configured in pyproject.toml for both black (line-length) and flake8 (max-line-length). Some of the lines were above 200 chars of length, so I simply ignored that warning.

Further, among these errors, I think the E7121 warning can be simply fixed by replacing == with is True and != with is not True.

Let me know if you think I can help with any of these.

Footnotes

  1. E712: comparison to True should be 'if cond is True:' or 'if cond:'

abzrg added 2 commits March 21, 2024 21:15
- Aside from 'flake8' itself, one extra optional dependency
  (Flake8-pyproject) was added as flake8 does not support pyproject.toml
  yet.
- For now I added all the existing flake8 warnings to 'ignore' entry in
  the pyproject.toml to be fixed later.
@abzrg
Copy link
Contributor Author

abzrg commented Mar 21, 2024

It seems that It has worked. At the end of CI log, it shows the count of errors, which is 0 for now.

@codecov-commenter
Copy link

codecov-commenter commented Mar 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.83%. Comparing base (3a3fa39) to head (48d79cd).

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #21   +/-   ##
=======================================
  Coverage   89.83%   89.83%           
=======================================
  Files           4        4           
  Lines         610      610           
=======================================
  Hits          548      548           
  Misses         62       62           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@gerlero
Copy link
Owner

gerlero commented Mar 21, 2024

LGTM. I've managed to remove the E712 exception but left the others (although those could be fixed too; I just couldn't do it now). Thanks again @abzrg!

@gerlero gerlero merged commit 90b1f2c into gerlero:main Mar 21, 2024
15 checks passed
@abzrg
Copy link
Contributor Author

abzrg commented Mar 21, 2024

Happy to help (:

@abzrg abzrg deleted the tool_lint branch March 21, 2024 21:59
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.

3 participants