ci: automatically test on all LTS node versions#134
Open
msimerson wants to merge 4 commits intomathiasbynens:mainfrom
Open
ci: automatically test on all LTS node versions#134msimerson wants to merge 4 commits intomathiasbynens:mainfrom
msimerson wants to merge 4 commits intomathiasbynens:mainfrom
Conversation
2220f66 to
daefa50
Compare
- with example for testing on extra versions - add node 21, as a "not LTS" example - ci: test windows too, for curiousity - ci: add coverage workflow - package.json: replace devDeps with npx
Author
|
Is there anything I can do to nudge this along? |
mathiasbynens
approved these changes
Apr 18, 2024
| "codecov": "^3.8.3", | ||
| "nyc": "^15.1.0", | ||
| "mocha": "^10.2.0" | ||
| }, |
Owner
There was a problem hiding this comment.
With these changes, you can no longer git clone + npm install to get everything you need to work offline. Please revert these changes.
Author
There was a problem hiding this comment.
Will do. Done.
Food for thought:
- the codecov module is officially deprecated
- nyc is almost unsupported (last release on npm, 4 years ago).
- nyc only supports es6 via babel transpilation to es5 😬
- the bare-bones support is has received is from bcoe, the author of c8, which remains the future of JS coverage reporting, using node's built-in coverage features.
- The GHA workflow uses nyc, for consistency
- the big coverage reporters (codecov, coveralls, etc.) are nudging everyone to move to GHA style processing
- the included GitHub action will report the coverage results in PRs
- moving the code coverage entirely to a GitHub Action improves the default install (
npm -i), because 99.9% 1 of the time, nobody benefits from installing dev modules they'll never use.npm ioutput now is 28 MB:added 237 packages, and audited 238 packages- w/o nyc & codecov:
6.4M ./node_modules - w/o nyc, codecov, & mocha:
0M ./node_modules
- I don't know about you but many of my PRs come from folks that still greatly struggle with git, let alone running
npm test. - if
mochais installed globally on your machine (as may be the case for a developer who prefers it), runningnpx mochawill use the globally installed (vianpm i -g) version. Not only does this work great offline, but it saves a lot of wasted developer time when doing installs and reinstalls of modules. - (aside: mocha is my favorite test runner), removing mocha as a dep pairs nicely with test: replace mocha with node.js builtin #135
- Made up statistic, but lets be honest: so few people used
npm i --productionthat all 68 of us noticed when npm changed it tonpm i --omit=dev
This file contains hidden or 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
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.
npxI'm happy to amend this PR in any way that suits the maintainer(s).
fixes #119
Previews