Switch to node8 & npm5 #2323
Merged
Switch to node8 & npm5 #2323
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- use official '8.9.4' docker images - checksum package-lock.json to check cache - replace (now unnecessary) install/dedupe/prune/install with a simple install (now with package-lock.json this does the right thing everytime ;)
... to ensure that we're bundling dist/ using node8/npm5
... as package-lock-json essentially does the same now.
alexcjohnson
reviewed
Feb 1, 2018
| @@ -65,6 +66,7 @@ | |||
| "no-use-before-define": [2, "nofunc"], | |||
| "no-loop-func": [2], | |||
| "no-console": [0], | |||
| "no-unused-labels": [2] | |||
| "no-unused-labels": [2], | |||
| "no-useless-escape": [0] | |||
There was a problem hiding this comment.
how badly do we break this rule?
There was a problem hiding this comment.
... and they are not auto-fixable (using eslint . --fix) unfortunately.
There was a problem hiding this comment.
... and I don't want to waste time with linting stuff. I really think we should be switching to standardjs and be done with it (cc #950) 😛
|
Looks great, lets do it! 💃 |
|
Ok. Merging this in. Devs, please make sure to update to node8/npm5 and |
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
resolves #1798
Tech-debt week continues ⛏️ 💰 🎉
This PR makes CI use node v8.9.4 and npm v5.6.0 (which works much better v3 😏 ). This CI image use Chrome 64 (i.e. the latest version) which should make it easier for us to make our tests run consistency locally and on CI.
Using npm5 gives us access to
package-lock.jsonfile (more info here) which contains a complete description of thenode_modules/tree. Now,npm i(should) give the same result everywhere everytime. This makes a few our tooling helpers obsolete:dist/npm-ls.jsonis gone and no need todedupeonpreversion.A few more things:
eslint@4(which runs a little faster now) andkarma@2,check-node-versionnow runs onpreversionensuring that whoever bundles thedist/use node8 and npm5.cc @alexcjohnson @dfcreative @nicolaskruchten @monfera