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

[Suggestion/Contribution]: Update to current Node LTS? #503

Open
MutableLoss opened this issue Feb 8, 2017 · 24 comments
Open

[Suggestion/Contribution]: Update to current Node LTS? #503

MutableLoss opened this issue Feb 8, 2017 · 24 comments

Comments

@MutableLoss
Copy link

MutableLoss commented Feb 8, 2017

I am very curious why this particular workshop/module is still based around an older version of Node, and why it even suggests that Linux users use an older version? I went through this entire workshop with 7.4.0 without any noticeable issues. I looked up the latest documentation, but the majority of those pages were applicable to very old versions of Node. It there a specific reason (besides time/contributions), that the current workshop is not based around 6.x / 7.x ?

I would love to contribute such changes to make the necessary changes needed to help with such an upgrade, but I definitely do not want to make any changes if the project has a specific model to follow which keeps this at an older version of Node.js.

@martinheidegger
Copy link
Contributor

@3desprit Thank you for pointing this out 💖 We try to have backwards compatibility to old versions of node but the documentation and information on learnyounode should be updated to newer versions. Any PR that updates learnyounode to match newer versions of Node is warmly welcome.

@MutableLoss
Copy link
Author

Huge thanks for the reply! I will get to working on a PR to help fix this.

@MutableLoss
Copy link
Author

So I am making sure to test the updated exercises in a various range of versions, specifically 0.10.48, 0.12.18, 4.7.3, 5.12.0, 6.9.5, and 7.5.0.

In regards to possible common versions, it does look like Ubuntu changed their "legacy" package in the Xenial and Yakkety builds over to 4.2.6, while systems like FreeBSD, and OpenBSD use the latest as their default Node.js port install. Even so, there are still package-manager options in various platforms that do offer a version as low as 0.10.25, but where does this project eventually want to cut ties with the old version, as Node.js has done itself with 0.10?

One of they main reasons I bring this up, is that the workshopper is starting to show minor deprecation errors with 7.5.0. This brings up the question, "When should the project itself limit itself to the most common versions, and not cater to scenarios where it's obvious the users knowingly chooses these older versions?"

@martinheidegger
Copy link
Contributor

Okay, thank you for being so thorough. I think the rule of thumb here should be usability: One year ago I remember that people with old versions of Node have still been coming to NodeSchool events and they struggled to install node with the old versions. (To be clear: 0.12+ ... haven't seen 0.10 in a while). That is why I kept the compatibility.

Deprecation errors are generally not helpful at events and its is possible to implement anything to work without errors and backwards compatible (any occurrence of a deprecation error is imho. a bug).

I think it would be a good step for the workshoppers if we gradually update the requirements to be compatible to every release within a year from that day and the LTS branch of one year back.
And if people run older versions of node: tell people how to update Node.js:.

Oh! I see you are running version {{node.version}} of Node.js! We do not test `{{appname}}` anymore and the sample code is made to take advantage of features of newer versions.

You need to upgrade node before continuing >> {{upgrade.url}}

(Something like that might be better, no?)

So this would cut the ties to 0.12 certainly. 4.75 would still be in there but several versions fall out. Let me open an issue in the org to see approval to this.

@MutableLoss
Copy link
Author

MutableLoss commented Feb 17, 2017

You are very welcome!

I definitely wanted to make sure to cover all of the bases to stay true to the project's overall mission.

Side note: Here's the issue for the deprecation error. workshopper/workshopper-adventure#82

Oh! I see you are running version {{node.version}} of Node.js! We do not test {{appname}} anymore and the sample code is made to take advantage of features of newer versions.
You need to upgrade node before continuing >> {{upgrade.url}}

(Something like that might be better, no?)

Oh man, that sounds like an amazing approach, and I cannot wait to see the outcome of that ticket.

In the meantime, I think I will just continue to work on a PR that covers the range of 0.12+, and dropping 0.10 since it sounds like it is not used unless explicitly installed.

Thanks a lot, that was much more than I ever expected. 😃

@martinheidegger
Copy link
Contributor

martinheidegger commented Feb 17, 2017

I think I wasn't quite clear: one year ago I saw 0.12. Today 0.12 should be mostly gone (right?) and I stand by the version suggestion in https://github.com/workshopper/org/pull/25/files#diff-bac9d9af11ebc4280f2a823f44731736R15 😉

@MutableLoss
Copy link
Author

No worries, that is totally my bad!

I have to agree. As far as I am aware, I believe Ubuntu's nodejs-legacy was the last well-known implementation of mass-available pre-iojs Node. Since node has made it quite easy to install the latest LTS via their own PPAs across various distros, I personally cannot see an issue. I could be wrong, but quickly skimming other distro's package repos, it does seem that 4.x is the lowest without explicitly installing something lower.

With that said, 4.3.2+. I can dig it. 😉

@MutableLoss
Copy link
Author

In reference to the documentation, do you want to offer the json files locally? If not, shall I at least remove the hyperlinks to the json counter-parts?

@martinheidegger
Copy link
Contributor

I am sorry: What json files are you talking about?

@MutableLoss
Copy link
Author

The official API doc's generate script now offers an option to show the document in JSON format (Alternate option next to View on single page). The only reason I can think of having this is for the sake of being complete, but would of course increase the size of the project for something new users probably would not use (unless something was added later to utilize it).

I don't mind either way, adding the JSON generated files, or simply removing the hyperlinks, but at first thought I would not feel comfortable submitting a PR with broken hyperlinks.

@martinheidegger
Copy link
Contributor

Oh! I didn't see that! (Nice feature I guess) I don't see a reason how the json variant would be useful offline. It would be nice to have a package.json script that pulls the documentation for the lowest supported Node.js version on pre-version.

@MutableLoss
Copy link
Author

I agree. There isn't too much use for it, except to create a document API.

That sounds like an awesome plan!

I will check out the scripts in the morning, and see if there is an easy non-conflicting way to accomplish that through a simple node script run through npm scripts.

At the moment the docs are at 7.5.0, but what about offering the ability for the user to match their node version as an argument in the script, or even have the script check the node version for them? The doc links could use a version based template variable, or even setup the doc template to exclude the version number, so the URLs would always be the same no matter what the version. Just a few spitball ideas.

@martinheidegger
Copy link
Contributor

The offline docs are only suggested or necessary or good when the user doesn't have an internet connection. Making sure that the version matches open a whole pandora of possible problems:
A) Should we download the latest or minimal or matching version on installation?
B) Should we download the latest or minimal or matching version on start?
C) Should we have a gzip cdn with all the docs?

etc.

I think that is too big of a pandora's box to open.

Thinking of practicability I guess it would be good to have a "node-offline-api" npm package that holds the various version. This should make both selection, matching, updating and the discussion easier. ;)

@MutableLoss
Copy link
Author

Ah yes, great point, and great idea! I will do some generation and packing tests tomorrow, and see how much space each version could take.

@MutableLoss
Copy link
Author

My apologies for the week of silence, I had an accident last Friday, and I was only released from the hospital yesterday. I can now get back to this contribution, but I did want to make it known I haven't forgotten it, or abandoned it.

@martinheidegger
Copy link
Contributor

@3desprit Take care, the work will not run away!

@MutableLoss
Copy link
Author

Quick update:

node-offline-api has been written, and I am now checking for edge cases before integrating into a PR to update the project.

@MutableLoss
Copy link
Author

So the document module is finally finished.

By default the module creates documents based on the currently running version of Node.js, and in the present working directory. In the case of learnyounode, it would create the docs in root of the learnyounode folder in a folder of its own "node-documents-<node version #>". Even though these are the defaults the version and folder can be set manually via script mode, and module mode.

With that said, would this be the preferred location for the docs? Can the projects have dynamic variables, or should the folder have a name without the version number? Are there any other files to consider with the document folder change?

@martinheidegger
Copy link
Contributor

For a first version updating the current docs as they are seems a solid step and having the build tool (node-offline-api?) to get the api might be nice as well.

@MutableLoss
Copy link
Author

Sounds great.

I got to thinking about this, and decided to add another option to the module that allowed for custom naming of the build folder. This way when installing the docs they can continue to be added directly to the apidoc folder, where links still point to the correct files, but with the advantage of the documents being built for v4.0.0 to latest.

I will make the change to the module, then create a PR for the added doc creation unless there is anything else you can think of?

@martinheidegger
Copy link
Contributor

Not at the moment. 👍

@MutableLoss
Copy link
Author

Well this has turned out to be a bit of a catch 22 when it comes to having the documents created with the project.

So far it works great, and does quite well how its setup to run it's own script when a condition is met when the docs do not exist, and when a flag is not passed to learnyounode.

The only issue is that it disallows the ability to pass in the Node 5 Travis test (local test passes with flying colors). The issue itself is based on a trivial and sporadic issue, because it running on startup which only runs when there aren't any flags (help, verify, etc).

I hate to make it another step in the installation process, but that's what it is starting to look like without getting into workshopper-adventure. Any thoughts or advice?

@martinheidegger
Copy link
Contributor

I am not sure why Node 5 travis fails but I am fairly certain that its not because of the docs 😉 I am rather busy these days, but it should really be a 1 step process. How else could this be fixed?

@MutableLoss
Copy link
Author

No worries at all on time, I'm in the middle of a big company rollout so I'm not in a hurry myself; most of all I wanted to plant the seed that this update is being held up due to the testing issue.

if there is some way to run it on install outside of the executable, that would most likely be ideal, but still leave the option there if someone needs to update their docs. Regardless, it would not need to be dependent on the startup.

The API takes care of everything (by design), so all that's needed is to call it at the opportune time. You can see exactly how this is done with the current setup here: 3941dec

Anyways,

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

No branches or pull requests

2 participants