Conversation
| npm-debug.log | ||
| node_modules | ||
| coverage | ||
| .nyc_output/ |
There was a problem hiding this comment.
This is from the istanbul->nyc change
| npm-debug.log | ||
| node_modules | ||
| coverage | ||
| .jshintrc |
There was a problem hiding this comment.
Copied from .gitignore, then adding a few things that probably shouldn't be in the tarfile sent to the npm repo.
| .travis.yml | ||
| .nyc_output/ | ||
| test/ | ||
| CHANGES.md |
There was a problem hiding this comment.
I'd be totally fine if you prefer CHANGES.md in the npm package.
| .jshintrc | ||
| .travis.yml | ||
| .nyc_output/ | ||
| test/ |
There was a problem hiding this comment.
Some people really like their tests in the package, but if the goal here is "small and tight" then they should be excluded IMO.
| - "14" | ||
| - "16" | ||
| - "18" | ||
| - "19" |
There was a problem hiding this comment.
I'm 90% sure that travis isn't going to run. Assuming it doesn't, I'd be happy to add GitHub actions to the repo.
| "pretest": "jshint *.js test", | ||
| "test": "istanbul test -- _mocha -R spec", | ||
| "posttest": "test -z $npm_config_coverage || istanbul report" | ||
| "test": "nyc -r lcov _mocha -R spec" |
There was a problem hiding this comment.
istanbul is deprecated, as is npm_config_coverage, which doesn't get set anymore. nyc is fast enough that it doesn't add enough overhead to the test loop to matter.
| }, | ||
| "engines": { | ||
| "node": ">=0.8.0" | ||
| "node": ">=14" |
There was a problem hiding this comment.
This is the breaking change, requiring a major version bump IMO. See https://github.com/nodejs/Release for justification.
1 participant
The most important change here is getting rid of
new Buffer. I'll do a quick code review to explain a few of my opinionated changes -- I'm happy to change anything you like, of course.I think I have previously signed the Strongloop CLA, although the link in CONTRIBUTING.md is now dead.