-
-
Notifications
You must be signed in to change notification settings - Fork 8.1k
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
Make the creation of a symlink an overridable default behaviour #514
Conversation
Since However, in the situation you describe, for |
Could you add tests for this behavior? (I'm glad your PR has the default be "on", and I'd prefer to think about changing the default in a future commit) |
In my use case, I do not want to allow anyone except superusers to install packages globally - but as I see it, it would be entirely possible to allow anyone to do package installs by giving the proper rights on the correct folder, so that pattern (superusers manage node versions, any user can npm install -g) would be doable too I think. I left the current behaviour on by default because this is what the current behaviour is - so I thought it would be better not to break backward compatibility with the currently released version here. As for tests, I am not sure how I could make it testable - unless I bail out explicitly when I fail to remove and re-create the symlink. Would that work for you? If so, I'll do that and write tests around |
@stelcheck See https://github.com/creationix/nvm/blob/master/test/fast/Running%20%22nvm%20use%20x%22%20should%20create%20and%20change%20the%20%22current%22%20symlink - a sibling test that asserts that when the env var is set, the symlink is not created, should cover it. The test would need to assert/ensure that the symlink didn't exist prior to the test, as well. |
Just made the test suite for that. Please let me know if its good enough or if I need to fix something. |
Nice, thanks! Interesting style. I'll review this and merge it tonight/tomorrow. |
Make the creation of a symlink an overridable default behaviour
The creation of a symlink kind of caused quite a bit of headaches because we build systems where NVM is installed (and managed) by superusers and used by a whole bunch of normal (or system) users.
This PR would allow us, on such system, to at least disable a feature that although doesn't technically hurt us or have much of an effect, generates errors on screen at login time or run time, like so:
(I know, I am using the future of Node :D)