chore: migrate to package.json-based linting package management#2576
chore: migrate to package.json-based linting package management#2576MikeMcC399 wants to merge 3 commits into
Conversation
Move version definition of npm packages to package.json & package-lock.json: - doctoc - markdown-link-check - prettier Update linting workflows. Enable npm updating through Dependabot. Add scripts and documentation to aid local linting.
| node-version: 'lts/*' | ||
| - name: Install markdown-link-check | ||
| run: npm i -g markdown-link-check@3.14.2 | ||
| node-version-file: .node-version |
There was a problem hiding this comment.
I think this is picked up automatically and doesn't need to be configured
There was a problem hiding this comment.
If neither node-version nor node-version-file is specified, then setup-node uses the default version of the runner, in this case ubuntu-latest => ubuntu-24.04 has Node.js 22.23.1 as default.
I did try it out, just to make sure. So this needs to stay as it is.
| "name": "docker-node", | ||
| "description": "Build official Docker images for Node.js", | ||
| "private": true, | ||
| "type": "commonjs", |
There was a problem hiding this comment.
We have a mix of Common and Module, note sure which is better to declare
There was a problem hiding this comment.
https://nodejs.org/docs/latest/api/packages.html#determining-module-system recommends to always include type.
Node.js always treats build-automation.mjs as module due to the mjs extension.
If type is undefined or set to commonjs then genMatrix.js and stackbrew.js are treated as commonjs
If type were to be set to module, then those files would be incorrectly treated as module, and running node stackbrew.js would trigger a ReferenceError.
The js extensions could be renamed to cjs for clarity. That could be done in a separate PR, or at some stage they could be migrated to ESM.
Anyway, the setting commonjs is correct here!
| "format:prettier": "prettier --write .", | ||
| "format:prettier:check": "prettier --check .", | ||
| "lint": "run-s format:toc format:prettier", | ||
| "check:markdown-links": "find *.md docs/*.md -print0 | xargs -0 -n1 markdown-link-check -c markdown_link_check_config.json" |
There was a problem hiding this comment.
The find stuff isn't cross platform portable, but I also struggled to get the globbing/ignore to work properly locally
| "private": true, | ||
| "type": "commonjs", | ||
| "scripts": { | ||
| "format:toc": "doctoc --update-only .", |
There was a problem hiding this comment.
Playing with this locally I needed additional parameters, but maybe it was a version issue. I had "doctoc:README": "doctoc README.md --github --title \"## Table of Contents\"", separate targets, otherwise I thing the title stuff was causing a diff
| - name: Install dependencies | ||
| run: npm ci | ||
| - name: Run Prettier | ||
| run: npx prettier --check . |
There was a problem hiding this comment.
| run: npx prettier --check . | |
| run: npm run format:prettier:check |
Better to call the configured target
| - name: Install dependencies | ||
| run: npm ci | ||
| - name: Run doctoc | ||
| run: npx doctoc --update-only . |
There was a problem hiding this comment.
| run: npx doctoc --update-only . | |
| run: npm run format:toc |
| "format:toc": "doctoc --update-only .", | ||
| "format:prettier": "prettier --write .", | ||
| "format:prettier:check": "prettier --check .", | ||
| "lint": "run-s format:toc format:prettier", |
There was a problem hiding this comment.
| "lint": "run-s format:toc format:prettier", | |
| "lint": "run-s format:toc format:prettier:check", |
I think you'd want the "check" versions" for this, but I also didn't know about the doctoc, if there was an equivilent.
2 participants
Description
Move version definition of npm packages to
package.json&package-lock.json:Use Node.js 24 (Active LTS).
Update linting workflows to run based on above package definitions:
workflow_dispatch:event triggers are added to make it easier to check linting issue before submitting a PR.Enable weekly npm updating through Dependabot.
Update CONTRIBUTING with prerequisites and linting instructions. Add Table of Contents.
Motivation and Context
The following workflows currently install npm dependencies on the fly. Code scanning alerts notes this as an issue.
latest)By moving the package version selection from GitHub Actions to npm
package.json&package-lock.jsonfiles, versions can be pinned exactly and maintained regularly by Dependabot.Setting Dependabot and npm minimum age to 5 days for updates, reduces the risk of fast supply-chain attacks:
Testing Details
Types of changes
Checklist