CFE-4632: Added python linting in github actions#122
CFE-4632: Added python linting in github actions#122victormlg wants to merge 3 commits intocfengine:masterfrom
Conversation
victormlg
force-pushed
the
linting
branch
11 times, most recently
from
5c9acec to
6db2a0c
Compare
victormlg
force-pushed
the
linting
branch
4 times, most recently
from
50474a2 to
387dc68
Compare
Signed-off-by: Victor Moene <victor.moene@northern.tech>
Signed-off-by: Victor Moene <victor.moene@northern.tech>
Signed-off-by: Victor Moene <victor.moene@northern.tech>
craigcomstock
left a comment
craigcomstock
left a comment
There was a problem hiding this comment.
sweet. thanks for the change to mostly in a script! Just one more nudge to move linting.sh to modules/ci directory. 👍
| - name: Check the formatting | ||
| run: cfbs --check pretty ./cfbs.json | ||
| - name: Linting | ||
| run: ./.github/workflows/linting.sh |
There was a problem hiding this comment.
Please put this in a modules/ci directory instead so it is more "visible" and standardized with our other repos.
| black --check . | ||
|
|
||
| echo "Running pyflakes" | ||
| pyflakes . No newline at end of file |
There was a problem hiding this comment.
Missing newline at the end of this file.
| try: | ||
| from ansible import context | ||
| from ansible.cli import CLI | ||
| from ansible.executor.playbook_executor import PlaybookExecutor | ||
| from ansible.inventory.manager import InventoryManager | ||
| from ansible.module_utils.common.collections import ImmutableDict | ||
| from ansible.parsing.dataloader import DataLoader | ||
| from ansible.plugins.callback import CallbackBase | ||
| from ansible.vars.manager import VariableManager | ||
| from ansible.plugins.loader import init_plugin_loader |
There was a problem hiding this comment.
This is a big change in behavior. The previous version would try to import and handle it if it doesn't work, i.e. if Ansible is not installed.
This is presumably because we don't know if Ansible is installed on the host where this runs (or is imported).
In this case, I think you should add an exception for this file, and not make such a big change. And if / when you're going to make that change, it should be in its own commit, with some more thorough explanation of why, why it's okay, how it is tested, how we are ensuring that Ansible is always importable, or similar.
3 participants
No description provided.