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

Effectively make stable an alias for lts. (Solves issue #335) #467

Merged
merged 1 commit into from Nov 5, 2018

Conversation

shadowspawn
Copy link
Collaborator

Pull Request Template:

Describe what you did

Made stable an alias for lts.

How you did it

Removed stable from documented options, other than as an alias for lts.
Modified display_latest_stable_version to return LTS version.
Retained existing stable code for backwards compatibility.

How to verify it doesn't effect the functionality of n

n --latest: 8.8.1
n --lts: 6.11.5
n --stable: 6.11.5
n latest && node --version: v8.8.1
n stable && node --version: v6.11.5
n latest && node --version: v8.8.1
n lts && node --version: v6.11.5

If this solves an issue, please put issue in PR notes.

Solves issue #335 where stable was using old pattern of latest even minor version number.

Using lts is a good match for "stable", but is a significant change to the erroneous behaviour which was sometimes same as latest .

If this solves an issue, please include the output of issue that had problems and then the fixed output from the same command.

Say node latest was at 8.9.2.

Old:
n --latest: 8.9.2
n --lts: 6.11.5
n --stable: 8.8.1

New:
n --latest: 8.9.2
n --lts: 6.11.5
n --stable: 6.11.5

Squash any unnecessary commits to keep history clean as possible

Place description for the changelog in PR so we can tally all changes for any future release

Change stable to be an alias for lts.

@shadowspawn
Copy link
Collaborator Author

Issue #354 also mentions no longer using Stable.

@shadowspawn
Copy link
Collaborator Author

A different approach to deprecation if preferred could be to remove stable from documentation, but retain existing behaviour. This has the potential advantage of changing no existing behaviour while still avoiding new users tripping over the old meaning of the stable label.

New references:
New user using n install stable [sic] mentioned in #485
Suggested terminology: #522

@troy0820 troy0820 self-requested a review September 4, 2018 13:25
Copy link
Collaborator

@troy0820 troy0820 left a comment

Choose a reason for hiding this comment

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

Merge conflicts but other than that this looks good.

@shadowspawn
Copy link
Collaborator Author

Rebased to resolve merge conflicts.

(Sorry about the merge conflict, due to whitespace changes, didn't notice I have my editor set to trim trailing spaces.)

@shadowspawn
Copy link
Collaborator Author

(If you wish, I will update Pull Request again to remove the two extraneous whitespace changes. Would have done it during the rebase if I had realised at the time.)

@troy0820 troy0820 merged commit 3366d06 into tj:master Nov 5, 2018
@shadowspawn shadowspawn deleted the issue/335-stable branch January 13, 2019 05:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants