Remove pylint process from github workflow #109
Conversation
|
Hey @hugodahl I think you're really close on this. Try the PR again, but remove all the commented out lines. |
|
@hugodahl I only see the removal of Pylint from the |
|
This should be a draft pull request. At least that's what I'd clicked when I created it! Wouldn't be my first time clicking the wrong button... on any day! |
|
No worries! I simply wanted to make sure I wasn't missing something. Thanks for putting it into draft status! |
The installation of pylint within the workflow is still needed, since the pre-commit execution of pylint is run through a shell call. Since it isn't being run through the dedicated pre-commit hook, pre-commit will not install pylint for us.
The pre-commit configuration file has had the calls to pylint for both the library code (./adafruit_*.py) and example code (./examples/*.py) added. The two calls are independant from one-another, and are named respectively "pylint-library" and "pylint-examples". The benefit of two separate calls is that they are easily distinguishable from one another, their rules are applied specifically to the correct patterns, and the comparison behaviour can be adjusted independantly (compare all files together as a batch versus individually). Additionally, a shell call was the only way to work around the pylint issue introduced in 2.7.0 which was causing duplicate code errors to fail the validation, which should not be an issue for the "examples/*.py" files, since each example should stand on its own.
05c794d to
0581192
Compare
|
I've made the initial change to have the pylint calls be done in pre-commit. However, given that there's a little time this evening, I would like to try and make the library code execution use the pre-commit hook for pylint, rather than the shell execution. This would help keep things neater and more in line with how we would expect pylint to work, if not for the current 2.7.0 issue with the |
...apitalize }}_{% endif %}CircuitPython_{{ cookiecutter.library_name}}/.pre-commit-config.yaml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
I tested this locally on my Mac and pushed the changes to my library and it ran successfully.
https://github.com/askpatrickw/CircuitPython_BloomSky/actions/runs/594815776
@kattni should review as well
...apitalize }}_{% endif %}CircuitPython_{{ cookiecutter.library_name}}/.pre-commit-config.yaml
Outdated
Show resolved
Hide resolved
Per discussion with @kattni in Discord (https://discord.com/channels/327254708534116352/327298996332658690/814157848400887809), the `bad-whitespace` check has been removed from pylint. To avoid future issues where it may be removed, we're pre-emptively removing it from our call. The check which was performed by this rule is now handled in "black".
|
Did this work out? I've been researching pylint & pre-commit and found that the recommendation is to NOT actually use I switched my own (non-circuitpython) project to a stanza like otherwise I got import-error diagnostics from pylint. |
This pull-request addresses issue #104, by moving the pylint process from being part of the GitHub workflow (from
.github/workflows/build.yml) to being part of the pre-commit workflow, so that it can be run on individual contributors' computers.