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

Miscellaneous fixes and updates #7

Merged
merged 3 commits into from
Jun 15, 2017
Merged

Miscellaneous fixes and updates #7

merged 3 commits into from
Jun 15, 2017

Conversation

tripu
Copy link
Member

@tripu tripu commented Jun 14, 2017

…some of them to adopt Node.js & npm's common/best practices.

  • Ignore node_modules/ directory and do not keep it under version control.
  • package.json:
    • Native Node.js modules should not be included.
    • Upgrade all other dependencies to latest versions (there were three updates available).
  • Node.js 8: include new file type package-lock.json (older versions of Node.js will ignore it).

and do not keep it under version control
Also, upgrade all dependencies to latest versions
@iherman
Copy link
Member

iherman commented Jun 14, 2017

@tripu, can you explain these changes a bit?

  • can a third party easily install the script if the packages are not around? I am not familiar enough with npm, I just followed the npm manuals (I was also surprised to see it installed the built in modules)
  • what is this node 8 file? I use nide version 6. Should I switch to version 8?

(The newbie mistakes with npm and node.js)

@tripu
Copy link
Member Author

tripu commented Jun 14, 2017

@iherman, sure, I'll try:

“Can a third party easily install the script if the packages are not around?”

Yes, it can. Those package names I removed from package.json are native to Node.js; included in all distributions. Any package that appears on the API docs is native. It's the first time I see them mentioned explicitly in package.json; I'm sure it's not the recommended way. The npm packages they point to seem either empty placeholders (eg, fs) or (outdated!) copies of the native ones (eg, path). So, definitely better not to have them there.

“What is this node 8 file? I use nide version 6. Should I switch to version 8?”

package-lock.json is a new format introduced in version 8, intended to “lock” all dependencies in an unambiguous, reproducible way. We are still learning to deal with it properly (cf discussion in w3c/specberus#602). We have migrated some of our Node.js tools and servers to 8 already. I think it's good to be on that version, but it's up to you :) When/if we have some automatic testing (#8), we can ask Travis CI to always test every change in both Node 6 and Node 8 (example; see the two “build jobs”), thus making sure the tool remains compatible with both environments.

@iherman
Copy link
Member

iherman commented Jun 15, 2017

@iherman, sure, I'll try:

“Can a third party easily install the script if the packages are not around?”

Yes, it can. Those package names I removed from package.json are native to Node.js; included in all distributions. Any package that appears on the API docs is native. It's the first time I see them mentioned explicitly in package.json; I'm sure it's not the recommended way. The npm packages they point to seem either empty placeholders (eg, fs) or (outdated!) copies of the native ones (eg, path). So, definitely better not to have them there.

O.k. What led me on the wrong path is looking at the npm documentation. There is a command that installs things locally and also changes the package.json file, and I did not realize it would blindly do it for packages that are built-in, too. My rookie mistake:-)

“What is this node 8 file? I use nide version 6. Should I switch to version 8?”

package-lock.json is a new format introduced in version 8, intended to “lock” all dependencies in an unambiguous, reproducible way. We are still learning to deal with it properly (cf discussion in w3c/specberus#602). We have migrated some of our Node.js tools and servers to 8 already. I think it's good to be on that version, but it's up to you :) When/if we have some automatic testing (#8), we can ask Travis CI to always test every change in both Node 6 and Node 8 (example; see the two “build jobs”), thus making sure the tool remains compatible with both environments.

Giving some second thoughts I think I would prefer to stay with Node 6. On the node.js site the download of Node 6 is still presented as "Recommended for most users" which I translate that Node 8 is still a bit experimental. Which also means that others who may want to download this script for local use (and I hope some people in the WG do that) may have only Node 6 installed on their local machine.

@iherman iherman merged commit 87ae1a1 into master Jun 15, 2017
@iherman iherman deleted the tripu/updates branch June 15, 2017 04:47
@tripu
Copy link
Member Author

tripu commented Jun 15, 2017

“Giving some second thoughts I think I would prefer to stay with Node 6.”

Okay. You can still test with both and try to maintain compatibility.

“On the node.js site the download of Node 6 is still presented as "Recommended for most users".”

Yes, it's still four months until version 8 becomes LTS.

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.

None yet

2 participants