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

Use Cabal to install shellcheck and upgrade to shellcheck v0.4.5 #1320

Merged
merged 2 commits into from
Nov 23, 2016

Conversation

PeterDaveHello
Copy link
Collaborator

No description provided.

1. This can help speed up the apt process since we decrease the
complexity of apt sources and package dependencies resolving by dropping
an additional ppa source.

2. Stack doesn't update ShellCheck to v0.4.5 after it has been released
more than one month, Cabal can provide ShellCheck v0.4.5
We need to use both the exit state and result form the previous commit,
so SC2181 should be disabled here.
@@ -269,6 +269,7 @@ nvm_ensure_version_installed() {
local NVM_VERSION_DIR
if [ "${EXIT_CODE}" != "0" ] || ! nvm_is_version_installed "${LOCAL_VERSION}"; then
VERSION="$(nvm_resolve_alias "${PROVIDED_VERSION}")"
# shellcheck disable=SC2181
Copy link
Member

Choose a reason for hiding this comment

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

Filed koalaman/shellcheck#782 to ask about this.

@ljharb
Copy link
Member

ljharb commented Nov 23, 2016

Is Cabal going to be faster than Stack here? This shellcheck job took 7:33, and the master one took 2:04, but I realize the caching hasn't kicked in yet.

@ljharb ljharb added the testing Stuff related to testing nvm itself. label Nov 23, 2016
@PeterDaveHello
Copy link
Collaborator Author

Of course once we use cache.

@ljharb ljharb merged commit 4b4e71f into nvm-sh:master Nov 23, 2016
@PeterDaveHello PeterDaveHello deleted the cabal-install-shellcheck branch November 23, 2016 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Stuff related to testing nvm itself.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants