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

Improve installation and upgrade instructions #1179

Merged
merged 4 commits into from
Jul 28, 2016

Conversation

lencioni
Copy link
Contributor

Ensure git describe gives latest tag

I recently ran the upgrade instructions and I ended up with the version
I was already on. This happened because git describe describes a
commit using the most recent tag reachable from it. Since I already had
a tag checked out, it was describing the tag I had already checked out.

Thankfully, git describe accepts an optional commit-ish, which it will
use instead of what we have currently checked out. Testing this in my
terminal now gives me the latest tag on origin, which is what I am
interested in when updating to the latest version.

  ~/.nvm ❯❯❯ git describe --abbrev=0 --tags
  v0.30.1
  ~/.nvm ❯❯❯ git describe --abbrev=0 --tags origin
  v0.31.3

I also added it to the manual install instructions for consistency and
extra safety.


Ensure git describe only matches version tags

git describe will match the latest tags, regardless of what it looks
like. We can make this a little safer by adding a --match flag to
match tags that look like version tags. This allows the maintainers of
this repo to more safely add other types of tags if they so wish,
without causing people to install or upgrade to those versions.


Use subshells for installation and upgrade instructions

I recently upgraded my copy of nvm and I was disappointed to be dropped
in the .nvm directory at the end of it. I also didn't like having to
copy and paste two separate blocks of code into my terminal, because I
missed the second one the first time around and was left in a slightly
confusing state. So, I decided to make this easier by utilizing
subshells and moving all of the instructions into one code block in this
document. I think this will improve people's experience maintaining this
tool.

To activate nvm, you need to source it from your shell:
1. clone this repo
2. check out the latest version
3. activate nvm by sourcing it from your shell
Copy link
Member

Choose a reason for hiding this comment

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

please use 1. throughout since ordered lists are autonumbered (for easier diffing/reordering)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh of course. I'll fix shortly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has been fixed now.

@ljharb ljharb added needs followup We need some info or action from whoever filed this issue/PR. informational installing nvm Problems installing nvm itself labels Jul 28, 2016
(
git clone https://github.com/creationix/nvm.git ~/.nvm
cd ~/.nvm
git checkout `git describe --abbrev=0 --tags --match "v[0-9]*" origin`
Copy link
Member

Choose a reason for hiding this comment

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

what versions of git support --match? I don't want to raise the minimum bar for git support if I can avoid it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I found it in the oldest version that the git documentation tells me about, 1.8.1.6. https://git-scm.com/docs/git-describe/1.8.1.6

I wanted an answer, so I cloned git and used git blame and traced it back to 30ffa603 from 2007. https://git.kernel.org/cgit/git/git.git/commit/?id=30ffa603 I found it mentioned in the release notes for git v1.5.5. https://git.kernel.org/cgit/git/git.git/tree/Documentation/RelNotes/1.5.5.txt

What is the current minimum bar for git support?

Copy link
Member

Choose a reason for hiding this comment

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

Sadly it's not documented - I'm just very cautious about adding new git arguments. That seems old enough that it won't be a concern.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed!

@lencioni lencioni force-pushed the improve-upgrade-instructions branch from 7fdeab4 to 99e41dd Compare July 28, 2016 17:08
@ljharb ljharb added needs rebase Please rebase your branch onto latest master! This removes merge commits, & keeps the git log clean. and removed needs followup We need some info or action from whoever filed this issue/PR. labels Jul 28, 2016
lencioni added 4 commits July 28, 2016 10:36
I recently ran the upgrade instructions and I ended up with the version
I was already on. This happened because `git describe` describes a
commit using the most recent tag reachable from it. Since I already had
a tag checked out, it was describing the tag I had already checked out.

Thankfully, `git describe` accepts an optional commit-ish, which it will
use instead of what we have currently checked out. Testing this in my
terminal now gives me the latest tag on origin, which is what I am
interested in when updating to the latest version.

  ~/.nvm ❯❯❯ git describe --abbrev=0 --tags
  v0.30.1
  ~/.nvm ❯❯❯ git describe --abbrev=0 --tags origin
  v0.31.3

I also added it to the manual install instructions for consistency and
extra safety.
`git describe` will match the latest tags, regardless of what it looks
like. We can make this a little safer by adding a `--match` flag to
match tags that look like version tags. This allows the maintainers of
this repo to more safely add other types of tags if they so wish,
without causing people to install or upgrade to those versions.
I recently upgraded my copy of nvm and I was disappointed to be dropped
in the .nvm directory at the end of it. I also didn't like having to
copy and paste two separate blocks of code into my terminal, because I
missed the second one the first time around and was left in a slightly
confusing state. So, I decided to make this easier by utilizing
subshells and moving all of the instructions into one code block in this
document. I think this will improve people's experience maintaining this
tool.
As suggested by @ljharb, this might be a little cleaner. I'm not
entirely sure, but in any case, it is consistent with the upgrade
instructions, so that is nice.
@lencioni lencioni force-pushed the improve-upgrade-instructions branch from ed4a9cd to 6eef4ce Compare July 28, 2016 17:37
@lencioni
Copy link
Contributor Author

Rebased.

@ljharb ljharb removed the needs rebase Please rebase your branch onto latest master! This removes merge commits, & keeps the git log clean. label Jul 28, 2016
@ljharb ljharb merged commit c874a17 into nvm-sh:master Jul 28, 2016
@lencioni lencioni deleted the improve-upgrade-instructions branch July 28, 2016 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
informational installing nvm Problems installing nvm itself
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants