Do not log when user has requested no profile modifications

[wesleytodd]

Currently when you run the following command it will not modify the profile, but it still logs a complaint and asks the user to add it.

$ PROFILE="/dev/null" ./install.sh
...
=> Profile not found. Tried  (as defined in $PROFILE), ~/.bashrc, ~/.bash_profile, ~/.zshrc, and ~/.profile.
=> Create one of them and run this script again
   OR
=> Append the following lines to the correct file yourself:
...

This changes the behavior so that explicitly setting to /dev/null does not nag you again. The change was simple so I figured I would submit a PR rather than an issue to discuss. Also, I looked at writing a test but having really no experience writing tests for a bash app like this I figured I should see if this seems good before spending too much time.

[wesleytodd]

I looked at the test failures and I am not sure what is up. I tried running them locally before pushing and aborted when it asked for my root password. Is this expected when running npm t?

[ljharb]

Yes, the tests do require sudo privileges.

[ljharb]

Looks like tests are failing due to a breaking change in a minor release: alanshaw/david#159

I'll try to work around that on master before rebasing this.

[ljharb]

For tests: the install script has all of its functions unit tested except for nvm_do_install (see https://github.com/nvm-sh/nvm/blob/master/test/install_script/nvm_do_install); and then has a few full tests like https://github.com/nvm-sh/nvm/blob/master/test/install_script/nvm_install_with_node_version and https://github.com/nvm-sh/nvm/blob/master/test/install_script/nvm_install_with_aliased_dot.

I think you could add a new test that explicitly tests the stdout/stderr output and asserts that the profile warning does, or does not, appear when expected?

[wesleytodd]

Awesome, so I am assuming that means you do like the change. I will work on tests for this when I have time. It might be a little bit due to Node Interactive and other things I have planned in Dec, but I won’t abandon it.

[wesleytodd]

but I won’t abandon it.

Famous last words!

For anyone else seeing this, I think this is a good change and it should just needs some bash tests. Feel free to take it over if you have time before I can get back to this.

[ljharb]

(Please don't open a new PR for that, though; please instead comment here with a link to a commit/branch, and I'll pull in the changes)

[wesleytodd]

Famous last words!

🤣

Honestly I was trying to push netflix to adopt nvm and this was a part of that. That work stalled out (reorgs, reprioritization, etc) and I am not sure I will be getting back to it any time soon. As I said above, anyone who wants to take this and run with it is welcome to.

[ljharb]

@wesleytodd keep me posted if there's anything i can do in the future to help that adoption.

[ljharb]

Finally added some tests.