Skip to content

Prow: Add shellcheck.sh task #95

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

Closed
alejovicu wants to merge 9 commits into from
Closed

Prow: Add shellcheck.sh task #95

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
8544432
test prow
alejovicu Jul 23, 2018
a6f6da1
just a test for prow
alejovicu Jul 30, 2018
2de30e4
adding a wrong shell to validate shellcheck
Jul 31, 2018
3ef29e3
fix the wrong shell to validate shellcheck
Jul 31, 2018
6873c58
break an sh to validate shell check
alejovicu Jul 31, 2018
e019847
fix sh to check if shellcheck is working
alejovicu Jul 31, 2018
ea5ba64
Prow: Add shellcheck.sh task
alejovicu Aug 1, 2018
b8a88a8
Prow: Update travis shellcheck task
alejovicu Aug 1, 2018
b5d934f
Prow: Add shellcheck to openshift/installer. Code reiew: Update shell…
Aug 2, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 4 additions & 9 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,12 @@ jobs:
name: Terraform lint
script: docker run -v $(pwd):/data -t quay.io/coreos/tflint
- script: >
docker run -t -v $(pwd):/workdir
quay.io/coreos/yamllint --config-data
'{extends: default, rules: {line-length: {level: warning, max: 120}}}'
docker run -t -v $(pwd):/workdir
quay.io/coreos/yamllint --config-data
'{extends: default, rules: {line-length: {level: warning, max: 120}}}'
./examples/ ./installer/
name: YAML lint
- script: >
docker run -v $(pwd):/workdir:ro
--entrypoint sh quay.io/coreos/shellcheck-alpine:v0.5.0
-c 'for file in $(find /workdir/ -type f -name "*.sh"); do
if ! shellcheck --format=gcc $file; then export FAILED=true; fi; done;
if [ "$FAILED" != "" ]; then exit 1; fi'
- script: ./hack/shellcheck.sh
name: shellcheck
- script: "docker run -v $PWD:/go/src/github.com/openshift/installer -w /go/src/github.com/openshift/installer quay.io/coreos/golang-testing go vet ./installer/..."
name: Go vet
Expand Down
2 changes: 2 additions & 0 deletions a-wrong-shell.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
#!/bin/sh
echo "this is a wrong shell"
10 changes: 10 additions & 0 deletions hack/shellcheck.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#!/bin/sh
Copy link
Member

@wking wking Aug 1, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file needs the executable bit set to avoid:

$ ./hack/shellcheck.sh
/home/travis/.travis/job_stages: line 78: ./hack/shellcheck.sh: Permission denied

DIR="${1:-.}"
if [ "$IS_CONTAINER" != "" ]; then
if find "${DIR}" -type f -name '*.sh' -exec shellcheck --format=gcc {} \+
then
echo "Shell Check Passed"
Copy link
Member

@wking wking Aug 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to print a success message. Exiting zero and the lack of warnings should be sufficient feedback. We also want to exit nonzero of the find exits nonzero. How about:

find "${DIR}" -type f -name '*.sh' -exec shellcheck --format=gcc {} \+ || exit 1

without the if? Even the || exit 1 may be optional, because with:

if [ "$IS_CONTAINER" != "" ]; then
  find "${DIR}" -type f -name '*.sh' -exec shellcheck --format=gcc {} \+
else
  docker run ...
fi

the find command will be the last command in the script, so the scripts default exit handling will pass it back to the caller.

Copy link
Contributor Author

@alejovicu alejovicu Aug 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This success message is to get an execution feedback from the pod log in Prow when it passed.

Copy link
Member

@wking wking Aug 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This success message is to get an execution feedback from the pod log in Prow when it passed.

If Prow needs something on stdout for success, you could always use:

./hack/shellcheck.sh && echo 'Shell Check Passed'

or similar in openshift/release#1131.

fi
else
docker run -e IS_CONTAINER='TRUE' --rm -v "$(realpath "${DIR}")":/workdir:ro --entrypoint sh quay.io/coreos/shellcheck-alpine:v0.5.0 /workdir/hack/shellcheck.sh /workdir;
fi;