-
Notifications
You must be signed in to change notification settings - Fork 371
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 shellcheck
and pre-commit
to CI jobs
#3002
base: master
Are you sure you want to change the base?
Conversation
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.
See comments and suggestions.
run: | | ||
pip install pre-commit | ||
|
||
- name: "Run shellcheck..." |
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.
- name: "Run shellcheck..." | |
- name: "Run pre-commit ..." |
The checks now are properly added and can be reviewed. The full review contains a ton of white-space changes, so I'd suggest to focus on changes introduced in ac35773 and the following first. There are a couple of intricate changes that were necessary to handle quoted strings correctly, including some bugs like |
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.
@terhorstd This looks good, but see the few inline comments. Did you run the shell scripts that have been modified significantly and which are not run by the CI, e.g., run_micrcocircuit and run_examples?
I am exploring the test failure on macOS. |
The reason for the MacOS failure is described in https://stackoverflow.com/a/752893. And I think I found a workaround. |
Works nicely on macOS now. |
This is an addendum to #2880 adding a
pre-commit
run also to the CI pre-build checks.The
shellcheck
is also added separately to address language specific linters on the same level.Related PRs are:
clang-format
pre-commit hook #2936isort
pre-commit to make in-place changes #2968