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

update ShellCheck to version 0.5.0 #1836

Open
wants to merge 5 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@sturman
Copy link

commented Jun 13, 2018

No description provided.

@ljharb

This comment has been minimized.

Copy link
Collaborator

commented Jun 13, 2018

Hmm, this should be failing tests, since there's still one more category of shellcheck checks that aren't passing on master.

@ljharb

This comment has been minimized.

Copy link
Collaborator

commented Jun 13, 2018

@PeterDaveHello any idea why these tests are passing, when I get this output locally?:

[Jordans-MacBook-Pro .nvm ((eabd7ab...)) v10.4.1 (1)]$ shellcheck -s bash nvm.sh && shellcheck -s sh nvm.sh && shellcheck -s dash nvm.sh

In nvm.sh line 53:
    INFO="$(which "${COMMAND}") ($(type "${COMMAND}" | command awk '{ $1=$2=$3=$4="" ;print }' | command sed -e 's/^\ *//g' -Ee "s/\`|'//g" ))"
            ^-- SC2230: which is non-standard. Use builtin 'command -v' instead.


In nvm.sh line 55:
    INFO="$(which "${COMMAND}") ($(type "${COMMAND}" | command awk '{ $1=$2=$3=$4=$5="" ;print }' | command sed 's/^\ *//g'))"
            ^-- SC2230: which is non-standard. Use builtin 'command -v' instead.

$ shellcheck -s ksh nvm.sh && shellcheck -s bash install.sh bash_completion nvm-exec

In nvm.sh line 53:
    INFO="$(which "${COMMAND}") ($(type "${COMMAND}" | command awk '{ $1=$2=$3=$4="" ;print }' | command sed -e 's/^\ *//g' -Ee "s/\`|'//g" ))"
            ^-- SC2230: which is non-standard. Use builtin 'command -v' instead.


In nvm.sh line 55:
    INFO="$(which "${COMMAND}") ($(type "${COMMAND}" | command awk '{ $1=$2=$3=$4=$5="" ;print }' | command sed 's/^\ *//g'))"
            ^-- SC2230: which is non-standard. Use builtin 'command -v' instead.
@ljharb
Copy link
Collaborator

left a comment

ah, just figured it out :-) you only updated the dockerfile, and travis-ci must still be on 0.4.7.

I'm going to keep this open until the 0.5.0 issues are resolved.

sturman added some commits Jun 14, 2018

@sturman

This comment has been minimized.

Copy link
Author

commented Jun 14, 2018

Updated shellcheck to v0.5.0 on Travis-CI. There is the same failed job as in #1836 (comment) on Travis-CI build

@PeterDaveHello
Copy link
Contributor

left a comment

I wonder if we should manually install the latest version or try to confirm/push Travis CI to update it? Their script was updated to fetch the latest version, maybe just need a small release to reflect that.

@ljharb

This comment has been minimized.

Copy link
Collaborator

commented Jun 14, 2018

I appreciate the expected failure here, but i agree - we should wait until travis has updated itself (which will happen on their next worker deploy).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.