build: Add Node 12, fix cache, discontinue Node 8 #159
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
* The Node 4 LTS reached end-of-life in April 2018. * The Node 6 LTS reached end-of-life in April 2019. * The Node 8 LTS reached enf-of-life in December 2019. <https://nodejs.org/en/about/releases/> <https://github.com/nodejs/Release#end-of-life-releases> This is done as preparation before other contributions I plan to make to update dependencies (some of which no longer support older versions), and some simplifications (some of which would require additional packages be added to support older versions). The previous versions of this package naturally conitinue to be available for those using them on older Node.js versions, and in the event of a critical bug we could even create an "v0.x-release" branch or something and create a new patch release from there against the older code.
* Remove 'sudo: false'. This was to make Travis use a fast container, instead of a VM. But, this option no longer exits. Travis now always uses a VM, and they're relatively fast. * Use 'cache: npm'. This repo has package-lock.json, which is great! (thank you). When package-lock.json exists, the installer deletes node_modules completely and rebuilds it fresh, based on the lock file. During local development, this is very fast because npm also has a cache at $HOME/.npm (which is not deleted). The 'cache: npm' shortcut will cache "$HOME/.npm" instead of "node_modules". Otherwise, this directory is copied by Travis from the cache but then immediately deleted by npm-install/npm-ci. <https://docs.travis-ci.com/user/languages/javascript-with-nodejs/#caching-with-npm> * Make 'npm test' include lint checks. This project uses ESLint and it can be run manually via 'npm run lint'. It is also run automatically by Travis. Make ESLint also executed by default when developers run 'npm test' locally. That makes it easier to contribute by not having to learn about lint issues after submitting a patch, and not having to read .travis.yaml or package.json in detail.
* Update from wikimedia eslint 0.8 to 0.15
- eslint-config-wikimedia now extends eslint, thus eslint is no
longer needed as explicit dependency.
- The eslint settings are now separate for browser JS and
Node.js. Update eslintrc to make this work correctly.
* Use 'Buffer.from(string, encoding)' instead of 'new Buffer()'.
The Buffer constructor has been deprecated as it had too manu
ambiguous purposes. In this case, the purpose was to convert
a utf-8 string, for which the explicit method is now Buffer.from().
> new Buffer() is deprecated. Use Buffer.from(), Buffer.alloc(),
> or Buffer.allocUnsafe() instead
> [no-buffer-constructor]
> https://nodejs.org/docs/v10.15.0/api/buffer.html#buffer_new_buffer_string_encoding
* Add line break in 'if (…) { … }'
> This line has 2 statements. Maximum allowed is 1
> [max-statements-per-line]
This is a style rule part of eslint-config-wikimedia which
has become stricter. The code mostly followed this style already,
except for a few cases in examples/, which have been updated
for consistency.
* Remove redundant '(function (){ …}())' in lib/ files.
This pattern is an Immediate-Invoked-Function-Expression (IIFE),
which exists mainly for the purpose of creating a private scope
for variables.
This is useful in client-side JS for a web browser, but not
in Node.js because every file already has its own private scope
by default.
This change might be easier to review by setting
"[x] Hide whitespace changes" in diff settings.
(Or add ?w=1 to the URL.)
|
@Krinkle - this look really good! Thanks! Will give it a more in-depth review soon and merge it. |
macbre
reviewed
Apr 12, 2020
|
@Krinkle - v0.13.0 puished to npm. Thanks once again for this change and your future plans regarding Looking forward to your next commits :) |
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.
Add Node 12 testing, discontinue Node 8 and earlier
https://nodejs.org/en/about/releases/
https://github.com/nodejs/Release#end-of-life-releases
This is done as preparation before other contributions I plan to make to update dependencies (some of which no longer support older versions), and some simplifications (some of which would
require additional packages be added to support older versions).
The previous versions of this package naturally conitinue to be available for those using them on older Node.js versions, and in the event of a critical bug we could even create an v0.x-release" branch or something and create a new patch release from there against the older code.
Minor CI changes
Remove 'sudo: false'. This was to make Travis use a fast container, instead of a VM. But, this option no longer exits. Travis now always uses a VM, and they're relatively fast.
Use 'cache: npm'.
This repo has package-lock.json, which is great! (thank you). When package-lock.json exists, the installer deletes node_modules completely and rebuilds it fresh, based on the lock file.
During local development, this is very fast because npm also has a cache at $HOME/.npm (which is not deleted).
The 'cache: npm' shortcut will cache "$HOME/.npm" instead of "node_modules". Otherwise, this directory is copied by Travis from the cache but then immediately deleted by npm-install/npm-ci.
https://docs.travis-ci.com/user/languages/javascript-with-nodejs/#caching-with-npm
Make 'npm test' include lint checks.
This project uses ESLint and it can be run manually via 'npm run lint'. It is also run automatically by Travis.
Make ESLint also executed by default when developers run 'npm test' locally. That makes it easier to contribute by not having to learn about lint issues after submitting a patch, and not having to read .travis.yaml or package.json in detail.