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

Explain --no-use in installation instructions #1839

Merged
merged 1 commit into from
Jun 18, 2018
Merged

Explain --no-use in installation instructions #1839

merged 1 commit into from
Jun 18, 2018

Conversation

JBallin
Copy link
Contributor

@JBallin JBallin commented Jun 17, 2018

No description provided.

README.md Outdated
@@ -59,6 +59,8 @@ export NVM_DIR="$HOME/.nvm"
[ -s "$NVM_DIR/nvm.sh" ] && \. "$NVM_DIR/nvm.sh" # This loads nvm
```

**Note:** Loading `nvm` is temporarily slow. While we work on a fix, you can add `--no-use` to the end of the above script (...`nvm.sh --no-use`). This will not load nvm until you actually `use` it (which you'll have to do manually, e.g. `nvm use default`).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to document the feature rather than what it's a workaround for. nvm isn't actually slow for most users; it's an unfortunate and vocal minority that experiences this.

README.md Outdated
@@ -59,6 +59,8 @@ export NVM_DIR="$HOME/.nvm"
[ -s "$NVM_DIR/nvm.sh" ] && \. "$NVM_DIR/nvm.sh" # This loads nvm
```

**Note:** You can add `--no-use` to the end of the above script (...`nvm.sh --no-use`) to postpone using `nvm` until you manually `use` it (e.g. `nvm use default`).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or just nvm use by itself :-)

This is great tho, I'll tweak and squash it prior to merging.

Copy link
Contributor Author

@JBallin JBallin Jun 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw you mention nvm use, but I get an error saying No .nvmrc file found followed by the nvm help output. My understanding is that the rc file needs to be created manually, so nvm use won't apply to all users.

Also, I'm happy to squash myself, if helpful. Guessing you want a single commit with a better commit message? Can make any other changes too. I'm no surgeon with rebase, but I'm pretty dangerous 😉.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nvm use also uses the default if available; nvm use default won't actually work for all users either.

nvm use node will work if there's a version installed - if not, nvm install node will both install and use node. I don't think there's a one-size-fits-all command, so maybe something generic that links to the usage section?

I'd probably go with the same commit message but prefixed with [Docs] :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Linked to usage. Let me know when satisfied with changes and then I'll squash and prefix with [Docs]

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! Please rebase this down to 1 commit on latest master; or if not I'm happy to do it for you as part of merging.

@JBallin
Copy link
Contributor Author

JBallin commented Jun 18, 2018

Opened PR #1840! Exciting to contribute to nvm, thanks for your support.

I decided to skip the rebase and just checkout the file from the other branch and commit to master. Is that a good workflow?

git checkout master
git checkout patch-1 README.md
git commit

@JBallin JBallin closed this Jun 18, 2018
@JBallin JBallin deleted the patch-1 branch June 18, 2018 04:39
@ljharb
Copy link
Member

ljharb commented Jun 18, 2018

No, the better workflow is never to open another PR. Can you please reopen this one?

@ljharb
Copy link
Member

ljharb commented Jun 18, 2018

I'm not going to merge this or #1840 until this is reopened; orphaned PR refs are an abomination.

@JBallin JBallin restored the patch-1 branch June 18, 2018 15:21
@JBallin
Copy link
Contributor Author

JBallin commented Jun 18, 2018

So sorry, didn't realize I could change the target branch to master! Can you help me figure out how to do that? Otherwise I can just squash down to one commit on the patch-1 branch?

@JBallin JBallin reopened this Jun 18, 2018
@ljharb
Copy link
Member

ljharb commented Jun 18, 2018

What was needed originally was to rebase the patch-1 branch and force push it. I've just force pushed your commit from #1840 to your patch-1 branch, so the two match, and I'll now merge them both at the same time.

@ljharb ljharb merged commit 41dc421 into nvm-sh:master Jun 18, 2018
@JBallin
Copy link
Contributor Author

JBallin commented Jun 18, 2018

Got it. So I should've kept everything on the same branch within the same PR.

Please rebase this down to 1 commit on latest master; or if not I'm happy to do it for you as part of merging.

I got confused by "master" but now realize you meant to be sure and pull down all the latest changes from master into my patch-1 branch (in case there were any commits since my last pull) before rebasing and squashing/fixing up the commits. Thanks for explaining and for all of your help! 🙏

@JBallin JBallin deleted the patch-1 branch June 18, 2018 23:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants