-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Add shellcheck to ci #12871
Add shellcheck to ci #12871
Conversation
ea0adb9
to
436413c
Compare
690b50c
to
cf959c2
Compare
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
a665861
to
c6b50be
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks for doing this.
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
wierdly, shellcheck is failing a script that is only present in master, and not in this PR ive disabled it for now, but im wondering why the script appeared and was being checked. |
This is expected, Azure pipelines (or GitHub Actions) runs CI on PR merge commit instead of PR head. |
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
Signed-off-by: Ryan Northey ryan@synca.io
Commit Message: Add shellcheck to ci
Additional Description:
Risk Level: low/medium
Testing:
Docs Changes:
Release Notes:
[Optional Runtime guard:]
[Optional Fixes #Issue] Partial fix for #7793
[Optional Deprecated:]