Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run shellcheck once in CI #128

Closed
wants to merge 7 commits into from
Closed

Conversation

szepeviktor
Copy link

@szepeviktor szepeviktor commented Jan 16, 2021

./lib/init/init.sh:38:13: error: Double quote array expansions to avoid re-splitting elements. [SC2068]
./lib/term/example.sh:41:1: error: Couldn't parse this brace group. Fix to allow more checks. [SC1073]
./lib/term/example.sh:44:9: error: '(' is invalid here. Did you forget to escape it? [SC1036]
./lib/term/example.sh:44:9: error: Expected a '}'. If you have one, try a ; or \n in front of it. [SC1056]
./lib/term/example.sh:44:9: error: Missing '}'. Fix any mentioned problems and try again. [SC1072]
./bpkg.sh:76:18: error: Double quote array expansions to avoid re-splitting elements. [SC2068]

May I fix the remaining ones?

@Potherca
Copy link
Member

That would be awesome! I did as many as I could before running out of steam, so any help is much appreciated!

I'm seeing some difference in the output of ludeeus/action-shellcheck

./lib/init/init.sh:38:13: error: Double quote array expansions to avoid re-splitting elements. [SC2068]
./lib/term/example.sh:41:1: error: Couldn't parse this brace group. Fix to allow more checks. [SC1073]
./lib/term/example.sh:44:9: error: '(' is invalid here. Did you forget to escape it? [SC1036]
./lib/term/example.sh:44:9: error: Expected a '}'. If you have one, try a ; or \n in front of it. [SC1056]
./lib/term/example.sh:44:9: error: Missing '}'. Fix any mentioned problems and try again. [SC1072]
./bpkg.sh:76:18: error: Double quote array expansions to avoid re-splitting elements. [SC2068]

and the output from pipeline-components/shellcheck

bpkg.sh:29:23: warning: Prefer mapfile or read -a to split command output (or quote to avoid splitting). [SC2207]
bpkg.sh:75:19: warning: Prefer mapfile or read -a to split command output (or quote to avoid splitting). [SC2207]
bpkg.sh:76:18: error: Double quote array expansions to avoid re-splitting elements. [SC2068]
bpkg.sh:95:27: warning: Prefer mapfile or read -a to split command output (or quote to avoid splitting). [SC2207]
bpkg.sh:97:20: warning: Expanding an array without an index only gives the first element. [SC2128]
setup.sh:46:5: warning: Use 'cd ... || exit' or 'cd ... || return' in case cd fails. [SC2164]
setup.sh:52:5: warning: Use 'cd ... || exit' or 'cd ... || return' in case cd fails. [SC2164]
setup.sh:81:20: note: Note that A && B || C is not if-then-else. C may run when A is true. [SC2015]
setup.sh:84:22: note: Note that A && B || C is not if-then-else. C may run when A is true. [SC2015]
setup.sh:113:14: note: Note that A && B || C is not if-then-else. C may run when A is true. [SC2015]
lib/init/init.sh:38:13: error: Double quote array expansions to avoid re-splitting elements. [SC2068]
lib/init/init.sh:142:16: warning: Prefer mapfile or read -a to split command output (or quote to avoid splitting). [SC2207]
lib/init/init.sh:152:7: warning: Variable was used as an array but is now assigned a string. [SC2178]
lib/install/install.sh:193:11: note: Check exit code directly with e.g. 'if mycmd;', not indirectly with $?. [SC2181]
lib/install/install.sh:367:7: warning: Variable was used as an array but is now assigned a string. [SC2178]
lib/install/install.sh:379:7: warning: Variable was used as an array but is now assigned a string. [SC2178]
lib/json/JSON.sh:188:14: warning: Expanding an array without an index only gives the first element. [SC2128]
lib/json/JSON.sh:188:41: warning: Expanding an array without an index only gives the first element. [SC2128]
lib/suggest/suggest.sh:73:18: warning: Prefer mapfile or read -a to split command output (or quote to avoid splitting). [SC2207]

I completely ignored lib/term/example.sh (as it is not production code), but other than that, I am not sure where the difference is coming from...

@szepeviktor
Copy link
Author

szepeviktor commented Jan 16, 2021

It must be shellcheck version.

Trying to help: ludeeus/action-shellcheck#36

@szepeviktor
Copy link
Author

I think they are v0.6.1 vs. v0.7.1 (ludeeus/action-shellcheck)

@szepeviktor
Copy link
Author

On your terminal the basic form of execution is shellcheck -S error $(find . -type f -name "*.sh" -not -path "./lib/term/example.sh")

@szepeviktor
Copy link
Author

There is no way to exclude a single file.
Please consider fixing ./lib/term/example.sh as I do not understand the intention of its tit-bits.

@Potherca
Copy link
Member

It must be shellcheck version.

pipeline-components/[email protected] runs ShellCheck 0.7.1, so I don't think that is it...

I just noticed one runs shellcheck with filtering out notes and warnings, the other doesn't. That is the difference. 🤦

Please consider fixing ./lib/term/example.sh as I do not understand the intention of its tit-bits.

Yeah, I also have no idea, which was part of my reason for skipping it...

@jwerle
Copy link
Member

jwerle commented Jan 19, 2021

We could just remove the lib/term/example.sh file as it exists upstream over here https://github.com/bpkg/term

@Potherca
Copy link
Member

Closing this in favor of #140 which also fixes these violations. 👍

@Potherca Potherca closed this Mar 27, 2022
@szepeviktor szepeviktor deleted the patch-1 branch March 27, 2022 16:01
@szepeviktor
Copy link
Author

szepeviktor commented Mar 27, 2022

Okay.
@Potherca Why do you waste CI resources? CI boots up for every single.
Theoretical question, I'm not ready to argue!

@Potherca
Copy link
Member

I'm already working on a run-once-on-all files setup, since almost all Shellcheck violations have been resolved in v1.

(Locally I'm only seeing eval warnings for ./lib/run/run.sh, everything else looks resolved...)

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

Successfully merging this pull request may close these issues.

3 participants