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

[shellcheck] update dockerfile to shellcheck v0.6.0 #1836

Merged
merged 1 commit into from Sep 27, 2019

Conversation

sturman
Copy link
Contributor

@sturman sturman commented Jun 13, 2018

No description provided.

@ljharb
Copy link
Member

ljharb 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
Copy link
Member

ljharb 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.

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

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
Copy link
Contributor Author

sturman 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

Copy link
Collaborator

@PeterDaveHello PeterDaveHello left a comment

Choose a reason for hiding this comment

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

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
Copy link
Member

ljharb 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).

@ljharb
Copy link
Member

ljharb commented Sep 27, 2019

locally, shellcheck is on v0.7.0; in travis it's v0.6.0.

I don't think this change is particularly needed anymore.

@ljharb ljharb force-pushed the update_shellcheck branch 2 times, most recently from c3bb04a to dcbecbf Compare September 27, 2019 21:47
@ljharb ljharb changed the title update ShellCheck to version 0.5.0 [shellcheck] update dockerfile to shellcheck v0.6.0 Sep 27, 2019
@ljharb ljharb merged commit dcbecbf into nvm-sh:master Sep 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants