Adding a symlink in .nvm to the current version. #467
Conversation
|
Duplicate of #447 |
|
Thanks for the contribution! However, I think #447's approach is more appropriate - this will show up in the output of In addition, this would need tests before I'd merge it. |
|
Thanks for the test! I think this could actually be a fast test - there's no need to actually do the install, just mkdir should be enough to verify. The bonus is that then it'd get tested on all the shells. |
|
Done @ljharb |
|
|
||
| . ../../nvm.sh | ||
|
|
||
| nvm install 0.10 |
There was a problem hiding this comment.
fast tests can't call nvm install - this should make a directory. Check out the other fast tests for examples. (same below)
|
My test is using the fast test already and should be good to go. Thoughts on it? |
|
@ljharb updated the test to not install. |
| @@ -542,6 +542,7 @@ nvm() { | |||
| export NODE_PATH | |||
| export NVM_PATH="$NVM_DIR/$VERSION/lib/node" | |||
| export NVM_BIN="$NVM_DIR/$VERSION/bin" | |||
| rm -f $NVM_DIR/current && ln -s $NVM_DIR/$VERSION $NVM_DIR/current | |||
There was a problem hiding this comment.
not a blocker, but it would be good to wrap each of the file paths in double quotes, in case there's any spaces or special chars in $NVM_DIR
|
It looks like your test is checking that the link is created, and changed - but not checking what it points to. Could you add checks that its path includes the expected version number in both cases? |
|
@ljharb done! |
| exit 1 | ||
| fi | ||
|
|
||
| if [ "$oldLink" == "$newLink" ];then |
There was a problem hiding this comment.
@ljharb this test is probably redundant now. Thoughts?
|
This looks great. I'll think on it a bit before merging, but I'm thinking I will soon :-) |
|
Thanks @ljharb! Please let me know if you'd like me to make any more updates. I look forward to having the symlink there in master for my c++ add-on development :) |
While working on some c++ add-ons, I thought it would be handy to have this change in my IDE.