[INFRA] Enable shellcheck on CI #4078
Conversation
test workflow
test1
fix spellcheck error
# Conflicts: # .github/workflows/style.yml
This reverts commit 39a9ff3
github-actions
bot
added
kind:build
kind:deploy
kind:infra
| @@ -100,3 +100,12 @@ jobs: | |||
| echo "If there is a problem that cannot be fixed by the command, " | |||
| echo "you need to manually fix it by following the information told by the command above" | |||
| echo "---------------------------------------------------------------------------------" | |||
| shellcheck: | |||
There was a problem hiding this comment.
please add a new line before it.
| @@ -227,6 +227,7 @@ Examples: | |||
| EOF | |||
| } | |||
|
|
|||
| # shellcheck disable=SC2199 | |||
There was a problem hiding this comment.
this is an acceptable approach, we can disable the check first, and fix them one by one in the future. (if the fix is simple, it's fine to fix in place in this PR as well)
| @@ -48,6 +49,7 @@ function kyuubi_rotate_log() { | |||
| num=${KYUUBI_MAX_LOG_FILES} | |||
| else | |||
| echo "Error: KYUUBI_MAX_LOG_FILES must be a positive number, but got ${KYUUBI_MAX_LOG_FILES}" | |||
| # shellcheck disable=SC2242 | |||
| exit -1 | |||
There was a problem hiding this comment.
this can be fixed in place, we just need to return a no-zero value here, 1 should be good.
There was a problem hiding this comment.
actually I try this in my fork project, the shell script has many grammar mistake, so I use #shellcheck jump that, I want fix these bug in issue #4057 before do this shellcheck
There was a problem hiding this comment.
what do you think about this idea
There was a problem hiding this comment.
the shell script has many grammar mistake
the shell script grammar is flexible; some are style issues, and should not be treated as bug/mistake, because they have worked as expected.
... fix these bug in issue #4057 before do this shellcheck
w/o shellcheck, how to verify the fix?
There was a problem hiding this comment.
en, I think I can fix some grammar mistake, then other problems we can use # to ignore them
|
Thanks, merged to master |
|
I'm pleasure that |
|
@xiaoyuandajian I just noticed that[1]
I opened the https://issues.apache.org/jira/browse/INFRA-24064 to ask the ASF INFRA team to evaluate this plugin and hope it can be added to the whitelist. [1] https://github.com/apache/kyuubi/actions/runs/3856320011 |
yes, I hope so |
…n folder ### _Why are the changes needed?_ <!-- fix-#4057 info: modify the shellcheck errors file in ./bin 1. "$" is a array, we want use string to compare. so modify "$" => "$*" 2. `tty` mean execute the command, we can use $(tty) replace it 3. param $# is a number, compare number should use -gt/-lt,not >/< 4. not sure the /bin/kyuubi line 63 'exit -1' need modify? so the directory bin only have a shellcheck note in /bin/kyuubi --> - fix shellcheck violations in scripts of /bin folder - enable shellcheck rule checks ### _How was this patch tested?_ - [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible - [ ] Add screenshots for manual tests if appropriate - [ ] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request Closes #4162 from davidyuan1223/master. Closes #4078 c48ad38 [yuanfuyuan] remove the used blank lines 55a0a43 [xiaoyuandajian] Merge pull request #10 from xiaoyuandajian/fix-#4057 cb11935 [yuan] Merge remote-tracking branch 'origin/fix-#4057' into fix-#4057 86e4e1c [yuan] fix-#4057 info: modify the shellcheck errors file in ./bin 1. "$@" is a array, we want use string to compare. so update "$@" => "$*" 2. `tty` mean execute the command, we can use $(tty) replace it 3. param $# is a number, compare number should use -gt/-lt,not >/< 4. not sure the /bin/kyuubi line 63 'exit -1' need modify? so the directory bin only have a shellcheck note in /bin/kyuubi dd39efd [袁福元] fix-#4057 info: 1. "$@" is a array, we want use string to compare. so update "$@" => "$*" 2. `tty` mean execute the command, we can use $(tty) replace it 3. param $# is a number, compare number should use -gt/-lt,not >/< Lead-authored-by: yuan <yuanfuyuan@mafengwo.com> Co-authored-by: 袁福元 <yuanfuyuan@mafengwo.com> Co-authored-by: xiaoyuandajian <51512358+xiaoyuandajian@users.noreply.github.com> Co-authored-by: yuanfuyuan <1406957364@qq.com> Signed-off-by: liangbowen <liangbowen@gf.com.cn>
Why are the changes needed?
Fix #2062
How was this patch tested?
Add some test cases that check the changes thoroughly including negative and positive cases if possible
Add screenshots for manual tests if appropriate
Run test locally before make a pull request