Skip to content

Run shellcheck once in CI - #128

Closed
szepeviktor wants to merge 7 commits into
bpkg:developfrom
szepeviktor:patch-1
Closed

Run shellcheck once in CI#128
szepeviktor wants to merge 7 commits into
bpkg:developfrom
szepeviktor:patch-1

Conversation

@szepeviktor

@szepeviktor szepeviktor commented Jan 16, 2021 •
edited
Loading

Copy link
Copy Markdown 
./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

Potherca commented Jan 16, 2021

Copy link
Copy Markdown
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

szepeviktor commented Jan 16, 2021 •
edited
Loading

Copy link
Copy Markdown
Author

It must be shellcheck version.

Trying to help: ludeeus/action-shellcheck#36

@szepeviktor

szepeviktor commented Jan 16, 2021

Copy link
Copy Markdown
Author

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

@szepeviktor

szepeviktor commented Jan 16, 2021

Copy link
Copy Markdown
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

szepeviktor commented Jan 16, 2021

Copy link
Copy Markdown
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

Potherca commented Jan 18, 2021

Copy link
Copy Markdown
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

jwerle commented Jan 19, 2021

Copy link
Copy Markdown
Member

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

@Potherca

Potherca commented Mar 27, 2022

Copy link
Copy Markdown
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

szepeviktor commented Mar 27, 2022 •
edited
Loading

Copy link
Copy Markdown
Author

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

@Potherca

Potherca commented Mar 27, 2022

Copy link
Copy Markdown
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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

See More

Successfully merging this pull request may close these issues.

3 participants

@szepeviktor @Potherca @jwerle