add shellcheck to CI

[MFTabriz]

• add shellcheck test [initially] with --severity=error to CI.
• fix errors!

shellcheck report for f3c7eb4 can be accessed from here.

[MFTabriz]

@drwetter in L22185 PROXY is defined as a string and not a variable. I was going to fix it but before that I need to know how that part is being tested?

[drwetter]

Thanks!

To add this is basically a very good idea however we need either to fix the errors or to mark them so that the CI passes.

In L22185 PROXY is defined as a string and not a variable. I was going to fix it but before that I need to know how that part is being tested?

Not sure I can follow you. $PROXY is a global. It is meant to be used as a string variable as it will contain, when defined, $PROXYIP:$PROXPORT

What do you mean by tested?

[MFTabriz]

To add this is basically a very good idea however we need either to fix the errors or to mark them so that the CI passes.

Yes! This is why this PR is still marked as a draft. :D

Not sure I can follow you. $PROXY is a global. It is meant to be used as a string variable as it will contain, when defined, $PROXYIP:$PROXPORT

$PROXY is indeed a variable but in this line we are using a literal string "PROXY" not the variable "$PROXY". [[ -n "PROXY" ]] will be always evaluated as true!

What do you mean by tested?

As this issue was not caught earlier, I’d expect the functionality testing is not working correctly [or maybe we are missing the coverage]. I’d rather fix the test first and then the code itself to check & document test functionality.

[drwetter]

[[ -n "PROXY" ]]`` will be always evaluated as true!

Oops, thanks for the clarification. :-) PR pending.

$DNS_VIA_PROXY needs also be true and it affects only the output. So, no, the output wasn´t tested in CI. Guess I tested that manually but obviously my eyes weren´t open enough.

[MFTabriz]

@drwetter I rebased this PR on #2436 and fixed the shellcheck errors.

[drwetter]

Ok, thanks!

CI went fine. However in some instances I am not sure yet whether the fixes are correct when changing from [@] to [*] Have you looked at the contexts whether it's correct to remove the expansion?

[drwetter]

Hi @MFTabriz ,

pls see my last comment.