Feat/shellcheck #1517
Feat/shellcheck #1517
Conversation
There was a problem hiding this comment.
I am working on rewriting the scripts to be proper Bash, and pass all shellcheck tests. Hence, I would prefer it if this PR is not merged. For instance, the logger.sh script already does not exist anymore and was replaced by logger.bash in #1246. Similarly, the startup.sh script saw a total rewrite in #1454 (which was reverted in #1561 but actually already has a proper fix in #1560 so that it can be merged again).
| - name: Setup Docker Environment | ||
| - name: Lint shell | ||
| run: | | ||
| make -C runner lint-scripts |
There was a problem hiding this comment.
Hey @CASABECI! Thanks for a lot for your contribution.
I finally managed to revisit this PR and have some time to integrate your work into the project,
and realized that we now have an exclusion rulle for runner/Makefile:
https://github.com/actions-runner-controller/actions-runner-controller/blob/f3fcb428ae56ee0a526cfffd35b7b24acd6e6704/.github/workflows/runners.yaml#L13
so if we added this lint-scripts target to the this file, it won't get triggered when the Makefile has changed.
Probably we'd better roll a new workflow dedicated for running spellcheck... Let me try.
I am about to revisit #1517, #1454, #1561, and #1560 as a part of our on-going effort to a major enhancement to the runner entrypoints being made in #1759. This change is a counterpart of #1852. #1852 enables you to easily run shellcheck locally. This enables you to automatically run shellcheck on every pull request. Currently, any shellcheck error does not result in failing the workflow job. Once we addressed all the shellcheck findings, we can flip the fail_on_error option to true and let jobs start failing on pull requests that introduces invalid or suspicious bash code.
|
|
||
| # SC1091: source /opt/bash-utils/logger.sh won't exist when shellcheck is ran | ||
| # shellcheck disable=SC1091 | ||
|
|
There was a problem hiding this comment.
FYI TIL that you can tell shellcheck the path to look from the scripts.
shellcheck --source-path runner made this mark unnecessary.
I am about to revisit #1517, #1454, #1561, and #1560 as a part of our on-going effort to a major enhancement to the runner entrypoints being made in #1759. This change is a counterpart of #1852. #1852 enables you to easily run shellcheck locally. This enables you to automatically run shellcheck on every pull request. Currently, any shellcheck error does not result in failing the workflow job. Once we addressed all the shellcheck findings, we can flip the fail_on_error option to true and let jobs start failing on pull requests that introduces invalid or suspicious bash code.
* Add workflow for validating runner scripts with shellcheck I am about to revisit #1517, #1454, #1561, and #1560 as a part of our on-going effort to a major enhancement to the runner entrypoints being made in #1759. This change is a counterpart of #1852. #1852 enables you to easily run shellcheck locally. This enables you to automatically run shellcheck on every pull request. Currently, any shellcheck error does not result in failing the workflow job. Once we addressed all the shellcheck findings, we can flip the fail_on_error option to true and let jobs start failing on pull requests that introduce invalid or suspicious bash code.
|
＠CASABECI Hey! Sorry for the massive delay, but we finally managed to introduce shellcheck into our CI workflow and fixed many lint errors along the way. See linked pull requests for more information. Your contribution enabled me to get started with shellcheck very quickly. Thanks a lot for your work! |
Thanks for resource available