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

[Docs] Install script - Improve installation guide #1544

Merged
merged 1 commit into from
Jun 3, 2017
Merged

[Docs] Install script - Improve installation guide #1544

merged 1 commit into from
Jun 3, 2017

Conversation

shivabhusal
Copy link
Contributor

@shivabhusal shivabhusal commented Jun 3, 2017

If installation is prompted to be successful, yet nvm command returns nvm: command not found then you might need to restart your terminal instance. Try opening a new tab/window in your terminal and retry.

If installation is prompted to be successful, yet `which nvm` command returns `not found` then you might need to restart your terminal instance. Try opening a new tab/window in your terminal and retry.
@ljharb
Copy link
Member

ljharb commented Jun 3, 2017

which nvm will always return not found, because nvm is not a binary or an executable; it's a sourced shell function. See https://github.com/creationix/nvm#verify-installation

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.

That said, this change looks good :-)

("restart your terminal" is exactly what the install script outputs to the terminal, but people don't read things)

@shivabhusal
Copy link
Contributor Author

shivabhusal commented Jun 3, 2017

@ljharb I updated the description. However, the which nvm command does not always return not found.

$ which nvm
nvm () {
	if [ $# -lt 1 ]
	then
		nvm --help
		return
	fi
	local COMMAND
	COMMAND="${1-}" 
	shift
	local VERSION
	local ADDITIONAL_PARAMETERS
	case $COMMAND in

        ...

@ljharb ljharb merged commit d2644b2 into nvm-sh:master Jun 3, 2017
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.

2 participants