add shellcheck action #42
Conversation
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
.github/workflows/shell-check.yml
Outdated
| fetch-depth: ${{ env.PR_FETCH_DEPTH }} | ||
|
|
||
| - name: shell-check | ||
| uses: YanxuanLiu/spark-rapids-common/shell-check@shellcheck |
There was a problem hiding this comment.
use nvidia repo instead of the personal one
shell-check/action.yml
Outdated
| # Get changed files | ||
| BASE_REF=$(git --no-pager log --oneline -1 | awk '{ print $NF }') | ||
| echo "Base REF: ${BASE_REF}" | ||
| FILES=$(git diff --name-only --diff-filter=AM ${BASE_REF} HEAD) || (echo "Your base commit ID is too old, please try upmerge first." && exit 1) |
There was a problem hiding this comment.
Can you please help explain why there will be an issue of 'base commit ID is too old',
This error is a little odd, so long as the PR can be submitted, there should not be such issue?
Another concern, we may only D(elete) files in a PR, but not A(dd) or M(odify) anything, then we should also not error out here, right?
|
Discussed offline, let's try to use the existing action if possible https://github.com/marketplace/actions/shellcheck |
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
|
Tested with existing The action is actually based on One limitation is that it cannot detect |
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
This PR adds a GitHub Actions workflow to automatically check shell script syntax and quality using both bash -n for syntax validation and shellcheck for comprehensive static analysis. The workflow aims to improve code quality by catching shell script issues early in the development process.
Key changes:
- Introduces automated shell script checking on pull requests
- Uses industry-standard tools for syntax and quality validation
- Configures appropriate PR event triggers and bot exclusions
|
Going to merge this one first. Please help update other repos and fix the existing issues if any in the same change thanks |
3 participants
fix #41
Use
bash -nandshellcheckto check shell script syntax.bash -n
This is a built-in Bash option that performs a syntax check without executing the script.
It only verifies if the script’s syntax is correct.
It does not detect logical errors, unsafe practices, or style issues.
shellcheck
shellcheck is a dedicated static analysis tool for shell scripts.
It checks for syntax errors and detects logical mistakes, potential bugs, unsafe code patterns, and style inconsistencies.
Provides detailed warnings and suggestions to improve code quality, security, and portability.
Supports multiple shell dialects including Bash, sh, and others.