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

fix(documentation): update all supported node version in README.md #28

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

arnaud-zg
Copy link

Linked issue #27

Copy link

@danhunsaker danhunsaker left a comment

Choose a reason for hiding this comment

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

Minor sorting issue to resolve - comments inline - and only one other issue. Please, here and in your boxfile.ymls, drop the revision number from all version strings. Major and minor are plenty for Node; some things only need the major. Most things shouldn't include a version number at all, but Node isn't one of them.

Anyway. Drop the revision parts of all the versions here, fix the sorting, and otherwise it looks good from here.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@arnaud-zg arnaud-zg force-pushed the fix/update-supported-node-version branch from de3521d to 81b9c64 Compare October 22, 2018 21:05
@danhunsaker
Copy link

Still need to drop revision numbers, but the sort looks better.

@arnaud-zg
Copy link
Author

Hum, what do you mean by drop revision numbers. I just checked, there is no package.json or CHANGELOG file in this repository.

@danhunsaker
Copy link

danhunsaker commented Oct 24, 2018

Version numbers for Semantic Verisoned projects, like Node, consist of a few parts. Namely, {Major}.{Minor}.{Revision}.{Patch}-{Tag}, where you can drop any component as long as you also drop the ones after it, though Patch is often omitted entirely anyway unless there's a hotfix.

The Nanobox packages for NodeJS include Major, Minor, Revision, and sometimes Tag values in their version numbers, but boxfile.yml configurations should not, so we intentionally leave out Revision (and Tag) values in the documentation.

@arnaud-zg
Copy link
Author

Got it, so actually I don't need to do anything on this pull request.

@arnaud-zg
Copy link
Author

About versioning, I actually use this package for my personal project. It can be a great idea to add it to this repository.

@danhunsaker
Copy link

@arnaud-zg Um, no, that's not what I was saying at all. Where you have 0.8.28 and 0.10.40, it should just be 0.8 and 0.10.

Is that clearer?

@arnaud-zg
Copy link
Author

Ooh okay, it's done. 😸

Copy link

@danhunsaker danhunsaker left a comment

Choose a reason for hiding this comment

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

Looks great!

@CruiseMan
Copy link

@danhunsaker Lets merge it, we need it

@arnaud-zg
Copy link
Author

Any news ?

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.

3 participants