Skip to content

ci: shellcheck: update to 0.8.0, fix/suppress new warnings#3371

Merged
thaJeztah merged 1 commit intoopencontainers:mainfrom
kolyshkin:bump-shellcheck
Feb 16, 2022
Merged

ci: shellcheck: update to 0.8.0, fix/suppress new warnings#3371
thaJeztah merged 1 commit intoopencontainers:mainfrom
kolyshkin:bump-shellcheck

Conversation

@kolyshkin
Copy link
Contributor

  1. This valid warning is reported by shellcheck v0.8.0:
	In tests/integration/helpers.bash line 38:
	KERNEL_MINOR="${KERNEL_VERSION#$KERNEL_MAJOR.}"
				       ^-----------^ SC2295 (info): Expansions inside ${..} need to be quoted separately, otherwise they match as patterns.

	Did you mean:
	KERNEL_MINOR="${KERNEL_VERSION#"$KERNEL_MAJOR".}"

Fix this.

  1. These (invalid) warnings are also reported by the new version:
	In tests/integration/events.bats line 13:
	@test "events --stats" {
	^-- SC2030 (info): Modification of status is local (to subshell caused by @bats test).

	In tests/integration/events.bats line 41:
		[ "$status" -eq 0 ]
		   ^-----^ SC2031 (info): status was modified in a subshell. That change might be lost.

Basically, this is happening because shellcheck do not really track
the call tree and/or local variables. This is a known (and reported)
deficiency, and the alternative to disabling these warnings is moving
the code around, which is worse due to more changes in git history.

So we have to silence/disable these.

  1. Update shellcheck to 0.8.0.

1. This valid warning is reported by shellcheck v0.8.0:

	In tests/integration/helpers.bash line 38:
	KERNEL_MINOR="${KERNEL_VERSION#$KERNEL_MAJOR.}"
				       ^-----------^ SC2295 (info): Expansions inside ${..} need to be quoted separately, otherwise they match as patterns.

	Did you mean:
	KERNEL_MINOR="${KERNEL_VERSION#"$KERNEL_MAJOR".}"

Fix this.

2. These (invalid) warnings are also reported by the new version:

	In tests/integration/events.bats line 13:
	@test "events --stats" {
	^-- SC2030 (info): Modification of status is local (to subshell caused by @BATS test).

	In tests/integration/events.bats line 41:
		[ "$status" -eq 0 ]
		   ^-----^ SC2031 (info): status was modified in a subshell. That change might be lost.

Basically, this is happening because shellcheck do not really track
the call tree and/or local variables. This is a known (and reported)
deficiency, and the alternative to disabling these warnings is moving
the code around, which is worse due to more changes in git history.

So we have to silence/disable these.

3. Update shellcheck to 0.8.0.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@kolyshkin
Copy link
Contributor Author

Centos7 ci failure is some unrelated glitch; filed #3372 to not forget.

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants