-
Notifications
You must be signed in to change notification settings - Fork 237
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
chore: Introduce shellcheck to lint .sh scripts #263
Conversation
This will fail on travis, as shellcheck finding were not fixed yet. Will propose on slack channel and if we want this I do followup commit with fixes to .sh files |
50d7e97
to
dd212b0
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.
I agree this should be added, but your initial version naturally fails the Travis build, so you’ll need to either fix up the scripts or turn off most warnings and start introducing them
Shellcheck would be nice given that this repo is all about scripts ! |
@grzesuav Merge conflict... |
279c9da
to
d147efb
Compare
@dinogun workin on this, but some changes are hard to verify without running. What is the best way to check that image generation is working (without the necessity to perform running all of them ) ? |
3cbe558
to
0320aeb
Compare
d26b6a0
to
95bdbfe
Compare
dfff9a8
to
f349b72
Compare
f349b72
to
a954643
Compare
418bdf8
to
306c2e4
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.
A few minor nits.
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.
LGTM. The build failures are unrelated to the PR.
No description provided.