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

Detect correct profile in install.sh based on $SHELL var #2556

Merged
merged 1 commit into from
Aug 21, 2021

Conversation

tg90nor
Copy link
Contributor

@tg90nor tg90nor commented Aug 12, 2021

This attempts to detect the users default shell rather than the shell the install script is running in, which will always be bash if following install instructions. Fixes #2555

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.

Thanks - could we add a test for this behavior?

install.sh Outdated Show resolved Hide resolved
install.sh Outdated Show resolved Hide resolved
@ljharb ljharb added the installing nvm: profile detection Issues with nvm's own install script, related to figuring out the right profile file. label Aug 12, 2021
@tg90nor
Copy link
Contributor Author

tg90nor commented Aug 12, 2021

Converted to single bracket test. I'll look into adding tests for this behaviour.

@tg90nor
Copy link
Contributor Author

tg90nor commented Aug 12, 2021

Updated the tests. Do you think it looks good now?

@tg90nor tg90nor requested a review from ljharb August 12, 2021 19:29
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.

This seems pretty good. I'm nervous about releasing it, because of the bug reports it might generate, but I suppose that'll surface what should go in the tests :-)

@ljharb ljharb force-pushed the better-install-detect-profile branch from 9d4917e to e98e9d9 Compare August 21, 2021 04:58
@ljharb ljharb merged commit e98e9d9 into nvm-sh:master Aug 21, 2021
@tg90nor tg90nor deleted the better-install-detect-profile branch September 29, 2021 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
installing nvm: profile detection Issues with nvm's own install script, related to figuring out the right profile file.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Shell detection fails in install script in some instances
2 participants