Skip to content
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 env BASH_SEVERITY to specify severity of errors in shellcheck for bash validation #3984

Merged
merged 2 commits into from
Apr 17, 2023

Conversation

bowenliang123
Copy link
Contributor

@bowenliang123 bowenliang123 commented Mar 11, 2023

Proposed Changes

Currently bash validation is executed with severity in the default value of style, which makes too many info level messages thrown and causes failures in using super-linter. For the users only cares about severity of warning and above, there's no way to config it in super-linter.

  1. support specifying minimum severity in shellcheck for bash validation by adding BASH_SEVERITY env
  2. make use of option --severity in shellcheck (https://github.com/koalaman/shellcheck/blob/master/shellcheck.1.md#options)

Readiness Checklist

Author/Contributor

  • If documentation is needed for this change, has that been included in this pull request

Reviewing Maintainer

  • Label as breaking if this is a large fundamental change
  • Label as either automation, bug, documentation, enhancement, infrastructure, or performance

@bowenliang123
Copy link
Contributor Author

cc @admiralAwkbar @ferrarimarco

@bowenliang123
Copy link
Contributor Author

Anybody help me to push this feature forward ? Like approval for CI jobs, and help me to pass the test. Thanks.

@bowenliang123
Copy link
Contributor Author

My changes don't affect to ansible-lint, but the execption encounters.

Anyone could help?

https://github.com/github/super-linter/actions/runs/4416946109/jobs/7764105838?pr=3984#step:7:243

  ×  super-linter-installed-commands: Super-Linter installed commands check (1 failed)
     ✔  Command: `command -v actionlint` exit_status is expected to eq 0
     ✔  Command: `actionlint --version` exit_status is expected to eq 0
     ✔  Command: `actionlint --version` stdout is expected to match /(.*?)/
     ✔  Command: `command -v ansible-lint` exit_status is expected to eq 0
     ✔  Command: `ansible-lint --version` exit_status is expected to eq 0
     ×  Command: `ansible-lint --version` stdout is expected to match /(.*?)/
     incompatible encoding regexp match (Windows-31J regexp with ASCII-8BIT string)

@lindluni
Copy link
Contributor

The ansible issues should be fixed now, apologies for how long this took, had to dig into those failures on main as well as a deprecated NPM package.

@ferrarimarco
Copy link
Collaborator

Why introduce this environment variables? Can't you set the severity in a .shellcheckrc file?

@bowenliang123
Copy link
Contributor Author

Why introduce this environment variables? Can't you set the severity in a .shellcheckrc file?

Negative.
.shellcheckrc rule file does not support severity directives, according to shellcheck's source code and wiki (https://www.shellcheck.net/wiki/Directive). The only way to set severity is command line option --severity= , referring to your link (https://github.com/koalaman/shellcheck/blob/master/shellcheck.1.md#rc-files)

@bowenliang123
Copy link
Contributor Author

bowenliang123 commented Apr 11, 2023

The ansible issues should be fixed now, apologies for how long this took, had to dig into those failures on main as well as a deprecated NPM package.

Thanks for the ping. And I have rebased my changes onto the latest commit of the master branch. Let's see whether it passes the CI builds.

@bowenliang123 bowenliang123 force-pushed the shellcheck-severity branch 2 times, most recently from 3419dde to d51a8a5 Compare April 17, 2023 01:22
@bowenliang123
Copy link
Contributor Author

Would you like to rerun CI builds and have some reviews or guidance on this PR, as it is submitted over one month ago ? @lindluni

@lindluni lindluni merged commit d301c72 into super-linter:main Apr 17, 2023
@bowenliang123 bowenliang123 deleted the shellcheck-severity branch April 17, 2023 03:56
@bowenliang123
Copy link
Contributor Author

Thanks for the merge and help from @lindluni and @ferrarimarco. Is this feature available in the github action only when it's released in the next tag?

@lindluni
Copy link
Contributor

Hi @bowenliang123

It will be available on the “latest” tag once this workflow completes: https://github.com/github/super-linter/actions/runs/4717260552

@lindluni
Copy link
Contributor

I will get a 5.0.1 published later this week for pinning purposes so the Action can be used directly

@bowenliang123
Copy link
Contributor Author

No problem. I will try to apply this feature in Apache Kyuubi for bash shell check when it's available in action.

@bowenliang123
Copy link
Contributor Author

bowenliang123 commented Apr 21, 2023

Hi @bowenliang123

It will be available on the “latest” tag once this workflow completes: https://github.com/github/super-linter/actions/runs/4717260552

Hi, a kind ping to @lindluni . It seems the latest tag still refer to v4 (v4.10.0) but not v5, when using github/super-linter/slim@latest in apache/kyuubi#4748 . see log: https://github.com/apache/kyuubi/actions/runs/4761101004/jobs/8462033635?pr=4748#step:2:1

Therefor, this feature is unavailable with the latest tag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants