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.

Sign up for GitHub

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

Jump to bottom

Use Shellcheck on CI for static analysis of Bash scripts #7793

Closed
derekargueta opened this issue Aug 1, 2019 · 3 comments · Fixed by #13106
Closed

Use Shellcheck on CI for static analysis of Bash scripts #7793

derekargueta opened this issue Aug 1, 2019 · 3 comments · Fixed by #13106
Assignees
@phlax
Labels
help wanted Needs help! tech debt

Comments

@derekargueta
Copy link
Member

Title: Use Shellcheck on CI for static analysis of Bash scripts

Description:
Shellcheck is a shell script analysis tool with numerous flags for atypical or dangerous bash constructs. Shellcheck's Gallery of Bad Code gives examples of what it catches. It's how I identified this minor bash issue in Envoy previously.

Shellcheck is additionally configurable so we can disable flags we don't want at the global level, file level, or line level for one-off exceptions. My team has been using Shellcheck and found it quite helpful. The only downside I know of is that it doesn't have a --fix flag like clang-tidy does.
@derekargueta
Copy link
Member Author

@mattklein123 please assign to me

@derekargueta derekargueta removed their assignment Apr 29, 2020
@phlax
Copy link
Member

phlax commented Aug 7, 2020

@mattklein123 im happy to take this

@dio dio assigned phlax Aug 7, 2020
@mattklein123
Copy link
Member

Nice! Thanks.

@phlax phlax mentioned this issue Aug 28, 2020
Merged
lizan pushed a commit that referenced this issue Sep 3, 2020
@phlax
Add shellcheck to ci (#12871)
c41bf57 
Add shellcheck to ci and fix some scripts that fail linting

This PR adds the framework for running shellcheck in ci

Im not sure if where i have hooked it in is the best place, apart from anything shellcheck does not give very reliable fixes - so it does not provide a patch to remedy as other code formatting ci does.

I have fixed some of the low-hanging fruit, but figured that it would probably be a good idea to fix other parts in separate PRs as they touch other parts of the code base

Additional Description:
Risk Level: low/medium
Partial fix for #7793 

Signed-off-by: Ryan Northey <ryan@synca.io>
This was referenced Sep 8, 2020
Merged
Merged
zuercher pushed a commit that referenced this issue Sep 11, 2020
@phlax
build: shellcheck tools/ (#13007)
0d4bcd8 
Risk Level: low
Testing: n/a
Docs Changes: n/a
Release Notes: n/a
Partial fix for #7793

Signed-off-by: Ryan Northey <ryan@synca.io>
This was referenced Sep 13, 2020
Merged
Merged
zuercher pushed a commit that referenced this issue Sep 17, 2020
@phlax
shellcheck: ci/ (#13024)
22f9592 
Risk Level: low
Testing: n/a
Docs Changes: n/a
Release Notes: n/a
Partial fix for #7793

Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax phlax mentioned this issue Sep 18, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@phlax phlax

Labels
help wanted Needs help! tech debt
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Shellcheck: bazel/ examples/ and other remaining linting issues phlax/envoy
3 participants
@phlax @derekargueta @mattklein123