ci: fix shellcheck warning SC2319 #2501
Conversation
github-actions
bot
added
github
.shellcheckrc
Outdated
|
|
||
| # SC2317: Command appears to be unreachable. Check usage (or ignore if invoked indirectly). | ||
| # https://github.com/koalaman/shellcheck/wiki/SC2317 | ||
| disable=SC2317 |
There was a problem hiding this comment.
I think this might be helpful... what is the reason to disable this?
There was a problem hiding this comment.
To resolve this we need to change 200+ lines of code as mentioned in #2272 (comment) .
I think this should be done as a follow-up PR (and likely will take a long time to resolve).
.shellcheckrc
Outdated
|
|
||
| # SC2086: Double quote to prevent globbing and word splitting. | ||
| # https://github.com/koalaman/shellcheck/wiki/SC2086 | ||
| disable=SC2086 |
There was a problem hiding this comment.
I do not see why would you want not to quote "$@". I'd rather solve this on case-by-case basis, as globally disabling this might lead to possibly faulty code.
There was a problem hiding this comment.
I'd rather solve this on case-by-case basis,
I do not see why this PR would stop you to help with this work and upload follow-up PRs that resolve this.
I do see that not having this PR land would enable us to introduce even more of this kind of problems as the current action-sh-schecker v0.6 does not report these.
| @@ -229,7 +229,8 @@ handle_netroot() { | |||
| echo "$target" | |||
| done | |||
| }) | |||
| [ -z "$targets" ] && warn "Target discovery to $iscsi_target_ip:${iscsi_target_port:+$iscsi_target_port} failed with status $?" && return 1 | |||
| ret=$? | |||
There was a problem hiding this comment.
It should be also declared as local.
There was a problem hiding this comment.
Wouldn't that trigger https://github.com/koalaman/shellcheck/wiki/SC3043 (it it would not be in shellcheckrc) ?
There was a problem hiding this comment.
Sure, you're right. I didn't know this file is supposed to be POSIX (and that posix doesn't know local). Prefixing it makes sense as well.
.shellcheckrc
Outdated
|
|
||
| # SC2004: $/${} is unnecessary on arithmetic variables. | ||
| # https://github.com/koalaman/shellcheck/wiki/SC2004 | ||
| disable=SC2004 |
There was a problem hiding this comment.
This one (SC2004 ) seems completely reasonable to me; why would you disable this?
|
@pvalena Thanks for the the reviews. You seem to indicate that this PR can only be landed by making massive changes all in one PR. Massive PRs like that do not get reviews for months. We can all point to a few PRs that are good but too big to review.
So that we can incrementally improve the code in reviewable increments. Making progress on this is blocked for about half year - see #2272 . At least this PR allows us to make progress and understand our current issues. |
|
This issue is being marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. If this is still an issue in the latest release of Dracut and you would like to keep it open please comment on this issue within the next 7 days. Thank you for your contributions. |
Henrik66
changed the title
fix shellcheck warning SC2319