-
Notifications
You must be signed in to change notification settings - Fork 513
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
GitHub Action to lint Python code #594
Conversation
These are a lot of steps if someone wants to run lint locally. Can we move it to a configuration file to simplify CI and local linting? |
The four lines from 10 thru 13 should be equivalent on a local machine. |
Because Travis CI seems to be on (permanent?) vacation... https://travis-ci.org/github/RobotWebTools/rosbridge_suite Related to RobotWebTools#593
.github/workflows/ci.yml
Outdated
steps: | ||
- uses: actions/checkout@v2 | ||
- uses: actions/setup-python@v2 | ||
- run: pip install --upgrade pip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this line is necessary? actions/setup-python
already gives us the latest version (see output from this on your branch)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not always... Watch it over time.
setup.cfg
Outdated
count = True | ||
max-complexity = 36 | ||
max-line-length = 228 | ||
select = E5,E9,F63,F7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- I think you need to add
C9
here for the cyclomatic complexity check to work? (at least I did on my machine - try settingmax-complexity
to something lower to see what I mean) - what do you think about including all
F
checks (and excluding the failing ones for now) like I did in https://github.com/RobotWebTools/rosbridge_suite/pull/605/files#diff-6951dbb399883798a226c1fb496fdb4183b1ab48865e75eddecf6ceb6cf46442R2-R4 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that to F issues should be fixed because PEP8 is clear about wildcard imports being a bad idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry I don't understand - can you rephrase?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Because Travis CI seems to be on (permanent?) vacation...
https://travis-ci.org/github/RobotWebTools/rosbridge_suite
Related to #593 and #600
https://flake8.pycqa.org/en/latest/user/error-codes.html
On the flake8 test selection, this PR does not focus on "style violations" (the majority of flake8 error codes that psf/black can autocorrect). Instead these tests focus on runtime safety and correctness: