-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
nvm: add page #7511
nvm: add page #7511
Conversation
Hello! I've noticed something unusual when checking this PR:
Is this intended? If so, just ignore this comment. Otherwise, please double-check the commits. |
The build for this PR failed with the following error(s):
Please fix the error(s) and push again. |
Apparently there's no equivalence of `nvm run` and `nvm exec` in the Windows version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nvm page is located in the common directory, you do not need to add it to the windows directory.
This is yet another different variant of NVM, since the original program does not run natively on Windows and some commands from the original (e.g. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @reinhart1010!
Since the original nvm
is completely incompatible with the version documented in common/nvm
, this is ok @einverne. See #7480
Left an optional comment below.
pages/windows/nvm.md
Outdated
# nvm | ||
|
||
> Install, uninstall or switch between Node.js versions. | ||
> Supports version numbers like "0.12" or "v4.2", and labels like "stable", "system", etc. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0.12
and v4.2
are seriously old anad unsupported versions of Node.js. Could we update these slightly at all please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I originally copied that from the original tldr-page of nvm
. Maybe we should update that too since this Windows version doesn’t support installing very old (io.js) versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @reinhart1010!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once my singular suggestion above is added then merge!
Co-authored-by: CleanMachine1 <78213164+CleanMachine1@users.noreply.github.com>
Done |
common/
,linux/
, etc.)Version of the command being documented (if known): 1.1.8
This is related to our discussion on #7480