Makefile: Ignore SC2329 #414
Conversation
It's not needed Signed-off-by: Paweł Gronowski <[email protected]>
In previous versions of shellcheck, SC2317 was emitted for every line of a unused function and we're ignoring the SC2317 completely: https://github.com/docker/docker-install/blob/6e61729e66af62fd54c0b659751a431be8f90f68/Makefile#L6 We were ignoring this error due to `version_compare` being reported as a false positive. After koalaman/shellcheck@4f81dbe this was fixed and now it produces one `SC2329` for the whole function instead of multiple SC2137. Signed-off-by: Paweł Gronowski <[email protected]>
| eval version_compare "$VERSION" "$1" | ||
| version_compare "$VERSION" "$1" |
There was a problem hiding this comment.
Is there any change in behavior between with / without eval ? (LOL, I may have written this code, but I honestly don't recall why it's there 🤔 🙈)
There was a problem hiding this comment.
It seems to work fine without it:
bash-3.2$ export VERSION=23.0
bash-3.2$ version_gte 24.0; echo $?
1
bash-3.2$ version_gte 34.0; echo $?
1
bash-3.2$ version_gte 23.0; echo $?
0
bash-3.2$ version_gte 23.5; echo $?
1
bash-3.2$ version_gte 20.10; echo $?
0
There was a problem hiding this comment.
Btw, the examples in the comment seem wrong 🙈
There was a problem hiding this comment.
Looks like a mistake in d08fb9e
-# VERSION=20.10
+# VERSION=23.0
+# version_gte 23.0 // 0 (success)There was a problem hiding this comment.
Wait, isn't it a holiday in NL? Go away Sebastiaan 😅
There was a problem hiding this comment.
Looks like a mistake in d08fb9e
-# VERSION=20.10 +# VERSION=23.0 +# version_gte 23.0 // 0 (success)
Isn't that correct though? I.e. 23.0 is "gte" (>=) 23.0, or didn't it work?
There was a problem hiding this comment.
Oh! Last line; I see it now; last line had to be updated because 21.10 would now no longer pass
Signed-off-by: Paweł Gronowski <[email protected]>
See #413