Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions .github/workflows/coverage.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
name: coverage

on:
push:
paths-ignore:
- '*.md'
pull_request:

env:
CI: true
NODE_ENV: cov

jobs:
coverage:
runs-on: ubuntu-latest
steps:
- uses: actions/setup-node@v4
- uses: actions/checkout@v4
- run: npm install
- name: run coverage
run: npx -y c8 --reporter=lcov npm test
- name: codecov
uses: codecov/codecov-action@v3
# - name: Coveralls
# uses: coverallsapp/github-action@master
# with:
# github-token: ${{ secrets.github_token }}
21 changes: 19 additions & 2 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,34 @@ on:

jobs:
build:
runs-on: ubuntu-latest
needs: [ get-lts ]
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
matrix:
os: [ ubuntu-latest, windows-latest ]
node:
- ${{ fromJson(needs.get-lts.outputs.active) }}
- 21
steps:
- name: Checkout
uses: actions/checkout@v4
- name: Set up Node.js
uses: actions/setup-node@v4
with:
node-version-file: '.nvmrc'
node-version: ${{ matrix.node }}
- name: Install dependencies
run: npm install
- name: Build
run: npm run build
- name: Test
run: npm test

get-lts:
runs-on: ubuntu-latest
steps:
- id: get
uses: msimerson/node-lts-versions@v1
outputs:
lts: ${{ steps.get.outputs.lts }}
active: ${{ steps.get.outputs.active }}
1 change: 0 additions & 1 deletion .nvmrc

This file was deleted.

7 changes: 1 addition & 6 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,9 @@
"punycode.es6.js"
],
"scripts": {
"test": "mocha tests",
"test": "npx mocha tests",
"build": "node scripts/prepublish.js"
},
"devDependencies": {
"codecov": "^3.8.3",
"nyc": "^15.1.0",
"mocha": "^10.2.0"
},
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With these changes, you can no longer git clone + npm install to get everything you need to work offline. Please revert these changes.

Copy link
Author

@msimerson msimerson Apr 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 i output 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 mocha is installed globally on your machine (as may be the case for a developer who prefers it), running npx mocha will use the globally installed (via npm 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
  1. Made up statistic, but lets be honest: so few people used npm i --production that all 68 of us noticed when npm changed it to npm i --omit=dev

"jspm": {
"map": {
"./punycode.js": {
Expand Down