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

chore: Introduce shellcheck to lint .sh scripts #263

Merged
merged 19 commits into from
Mar 5, 2020

Conversation

grzesuav
Copy link
Contributor

No description provided.

@grzesuav
Copy link
Contributor Author

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

@grzesuav grzesuav force-pushed the add_shellcheck branch 2 times, most recently from 50d7e97 to dd212b0 Compare January 11, 2020 20:28
@karianna karianna added this to the January 2020 milestone Jan 12, 2020
Copy link
Member

@karianna karianna left a 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

@dinogun
Copy link
Collaborator

dinogun commented Jan 16, 2020

Shellcheck would be nice given that this repo is all about scripts !

@karianna
Copy link
Member

@grzesuav Merge conflict...

@grzesuav
Copy link
Contributor Author

@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 ) ?

10/jdk/alpine/slim-java.sh Outdated Show resolved Hide resolved
Copy link
Collaborator

@dinogun dinogun left a 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.

slim-java.sh Outdated Show resolved Hide resolved
slim-java.sh Outdated Show resolved Hide resolved
slim-java.sh Outdated Show resolved Hide resolved
Copy link
Collaborator

@dinogun dinogun left a 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.

@dinogun dinogun merged commit 8a90660 into AdoptOpenJDK:master Mar 5, 2020
@grzesuav grzesuav deleted the add_shellcheck branch March 11, 2020 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants