ci(action): add shellcheck linter #6340
Conversation
- action-shellcheck@1.1.0 Signed-off-by: kwanhur <huang_hua2012@163.com>
| @@ -7,3 +7,6 @@ | |||
| [submodule "t/grpc_server_example"] | |||
| path = t/grpc_server_example | |||
| url = https://github.com/api7/grpc_server_example | |||
| [submodule ".github/actions/action-shellcheck"] | |||
| path = .github/actions/action-shellcheck | |||
| url = https://github.com/ludeeus/action-shellcheck | |||
There was a problem hiding this comment.
Hi, Why do you add this into the submodule?
There was a problem hiding this comment.
I think it's due to policy of ASF? Actions not from Github or ASF are forbidden.
|
Refers to PR#5364, only actions from GitHub or Apache can be used in the CI under Apache's policy.
So it's used with submodule.
Kwanhur Huang
TL;DR
… 2022年2月16日 下午8:45，Peter Zhu ***@***.***> 写道：
@starsz commented on this pull request.
In .gitmodules <#6340 (comment)>:
> @@ -7,3 +7,6 @@
[submodule "t/grpc_server_example"]
path = t/grpc_server_example
url = https://github.com/api7/grpc_server_example
+[submodule ".github/actions/action-shellcheck"]
+ path = .github/actions/action-shellcheck
+ url = https://github.com/ludeeus/action-shellcheck
Hi, Why do you add this into the submodule?
—
Reply to this email directly, view it on GitHub <#6340 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AANVRBSRKWZ4I36DNHZTH5TU3OL63ANCNFSM5OQX4MDA>.
Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you authored the thread.
|
Thanks for your patient explanation.Got it. |
| @@ -1,3 +1,5 @@ | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
The common.sh is imported in other scripts, so there is no need to add #!.
| @@ -1,3 +1,5 @@ | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
The common.sh is imported in other scripts, so there is no need to add #!.
|
The shellcheck action is imported but doesn't run at all... I think we should write our own GitHub action, like what we have done with editorconfig action: #5391 The submodule way doesn't obey ASF's policy "Actions not from Github or ASF are forbidden.". It just bypasses the restriction set in the CI. The policy maker of ASF won't be happy to see this. |
|
A PR is submitted to remove the tmate action finally: #6367 |
|
From the context, make one I'll stop working on this PR under confirmation. |
|
I am afraid we can't create a repo for shellcheck under Apache's GitHub organization. We need to apply for the new repo and a fork of shellcheck action may not be acceptable. I think just copying some code to the lint action is enough. |
|
Copying codes or code files, these all need to reserve license declaration, will it have any conflicts? action-shellcheck and shellcheck under MIT license |
|
So we need to write our own installation to add shellcheck in the CI. |
|
I'll try it. Thanks for the guidance. |
|
After a trial, using a local action works fine, have a look at shellcheck action and test case: In ci/common-test.sh line 1:
. .common.sh
^-- SC2148 (error): Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
For more information:
https://www.shellcheck.net/wiki/SC2148 -- Tips depend on target shell and y...
Error: Process completed with exit code 1.Summary, two solutions can do the shellcheck: I prefer solution1, what's your opinion, @spacewander ?
scversion="latest"
wget -O- "https://github.com/koalaman/shellcheck/releases/download/${scversion?}/shellcheck-${scversion?}.linux.x86_64.tar.xz" | tar -xJv
cp -av "shellcheck-${scversion}/shellcheck" /usr/local/bin/
shellcheck --version
shellcheck **/*.sh
runs:
using: composite
steps:
- id: shellcheck
run: |
scversion="${{ inputs.scversion }}"
wget -O- "https://github.com/koalaman/shellcheck/releases/download/${scversion?}/shellcheck-${scversion?}.linux.x86_64.tar.xz" | tar -xJv
cp -av "shellcheck-${scversion}/shellcheck" /usr/local/bin/
shellcheck --version
shellcheck **/*.sh
shell: bash |
|
I vote for the first solution. |
|
Solution1 implements with #6428 This PR closed :-) |
What this PR does / why we need it:
Pre-submission checklist: