CI: improvements - #271
Merged
Merged
Conversation
thackara
force-pushed
the
ci_improvements
branch
2 times, most recently
from
May 25, 2026 07:37
87f6314 to
8dfe17f
Compare
ddiss
requested changes
May 27, 2026
ddiss
left a comment
ddiss
left a comment
Collaborator
There was a problem hiding this comment.
Thanks Sangeetha. The changes look fine, with a couple of minor suggestions below
thackara
force-pushed
the
ci_improvements
branch
6 times, most recently
from
June 2, 2026 10:18
45d86e3 to
1e39147
Compare
address failures caught by `shellcheck`
|
| In autorun/fcoe_local.sh line 121:
| while [ ! -d /sys/class/fc_remote_ports/rport-* ]; do
| ^-- SC2144 (error): -d doesn't work with globs. Use a for loop.
|
|
| In autorun/fcoe_local.sh line 133:
| for b in "${target}/${hctl}/block/*"; do
| ^-------------------------^ SC2066 (error): Since you double quoted this, it will not word split, and the loop will only run once.
|
|
| In autorun/fstests_ntfs3.sh line 11:
| ln -s "${mkfsbin}" "${mkfsbin}3" || break
| ^---^ SC2105 (error): break is only valid in loops.
|
|
| In autorun/ocfs2.sh line 39:
| for i in $(ls /sys/block); do
| ^--------------^ SC2045 (error): Iterating over ls output is fragile. Use globs.
|
|
| In autorun/openiscsi.sh line 39:
| for i in $(ls /etc/iscsi/nodes/*/*/default); do
| ^-- SC2045 (error): Iterating over ls output is fragile. Use globs.
|
|
| In cut/fstests_xfs.sh line 17:
| [[ ${man_deps[@]} == ${man_deps[@]%.gz} ]] || man_deps+=(zcat gzip)
| ^------------^ SC2199 (error): Arrays implicitly concatenate in [[ ]]. Use a loop (or explicit * instead of @).
| ^----------------^ SC2199 (error): Arrays implicitly concatenate in [[ ]]. Use a loop (or explicit * instead of @).
|
|
| In cut/fstests_xfs.sh line 18:
| [[ ${man_deps[@]} == ${man_deps[@]%.bz2} ]] || man_deps+=(bzcat)
| ^------------^ SC2199 (error): Arrays implicitly concatenate in [[ ]]. Use a loop (or explicit * instead of @).
| ^-----------------^ SC2199 (error): Arrays implicitly concatenate in [[ ]]. Use a loop (or explicit * instead of @).
|
|
| In cut/fstests_xfs.sh line 19:
| [[ ${man_deps[@]} == ${man_deps[@]%.xz} ]] || man_deps+=(xzcat)
| ^------------^ SC2199 (error): Arrays implicitly concatenate in [[ ]]. Use a loop (or explicit * instead of @).
| ^----------------^ SC2199 (error): Arrays implicitly concatenate in [[ ]]. Use a loop (or explicit * instead of @).
|
|
| In selftest/selftest.sh line 60:
| for t in $(ls "selftest/test/"${filter}); do
| ^-----------------------------^ SC2045 (error): Iterating over ls output is fragile. Use globs.
|
| For more information:
| https://www.shellcheck.net/wiki/SC2045 -- Iterating over ls output is fragi...
| https://www.shellcheck.net/wiki/SC2066 -- Since you double quoted this, it ...
| https://www.shellcheck.net/wiki/SC2105 -- break is only valid in loops.
Changes include:
- Replace `ls` output with safe bash globs SC2045
- Fixed path resolution when migrating away from `ls` basenames.
- Fixed `break` (SC2105).
- Replaced implicit array concatenation in `[[ ]]` tests with explicit
expansion syntax (SC2199).
- Replaced unsafe dir glob inside `[ -d ]` SC2144.
- Fixed SC2166.
Signed-off-by: Sangeetha Thackarajan <sangeetha.thackarajan@suse.com>
this is purely formatting change with no functional alterations (done in preparation for new CI/lint job). cmd: `git ls-files "*.rs" | grep -v "^src/third_party" | xargs rustfmt` Signed-off-by: Sangeetha Thackarajan <sangeetha.thackarajan@suse.com>
Signed-off-by: Sangeetha Thackarajan <sangeetha.thackarajan@suse.com>
thackara
marked this pull request as ready for review
June 2, 2026 11:33
Contributor
Author
Thanks David, I have addressed the suggestions. The implementation now passes shellcheck, rustfmt, and clippy lints (with warnings allowed). Do you have any further comments, suggestions, or questions I should address? |
Collaborator
All looks good to go in Sangeetha :) I just wanted to have a closer look at the |
ddiss
approved these changes
Jun 9, 2026
| - name: Run Rust format check and clippy lint | ||
| run: | | ||
| cargo --version | ||
| cargo fmt --check --all |
Collaborator
There was a problem hiding this comment.
This differs slightly from c1e3994 in that you don't filter out third_party files. Is there any reason why?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Raising this PR to get inputs.