Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: improve setup & CI config #6

Merged
merged 16 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
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
15 changes: 15 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
root = true

[*]
charset = utf-8
end_of_line = lf
indent_style = tab
insert_final_newline = true
max_line_length = 100
trim_trailing_whitespace = true

[*.json]
indent_style = space

[*.{yaml,yml}]
indent_style = space
1 change: 1 addition & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@nodejs/userland-migrations
46 changes: 28 additions & 18 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,28 @@ on:

jobs:
lint-and-types:
if: github.event.pull_request.draft == false

runs-on: ubuntu-latest

strategy:
matrix:
node-version: [23.x]
node:
- version: 23.x
steps:
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # 4.2.2
with:
node-version: ${{ matrix.node-version }}
cache: 'npm'
- run: npm ci
- run: npm run lint
- run: npm run format
- run: npm run test:types
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # 4.2.2
- name: Set up Node.js ${{ matrix.node.version }}
uses: actions/setup-node@39370e3970a6d050c480ffad4ff0ed4d3fdee5af # 4.1.0
with:
node-version: ${{ matrix.node.version }}
cache: 'npm'
- run: npm ci
- run: node --run lint
- run: node --run type-check

tests:
if: github.event.pull_request.draft == false

strategy:
fail-fast: false
matrix:
Expand All @@ -41,12 +47,16 @@ jobs:
runs-on: ${{ matrix.os }}

steps:
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # 4.2.2
- name: Set up Node.js ${{ matrix.node.version }}
uses: actions/setup-node@39370e3970a6d050c480ffad4ff0ed4d3fdee5af # 4.1.0
with:
node-version: ${{ matrix.node.version }}
cache: 'npm'
- run: npm ci
- run: npm run test:unit
- run: npm run test:e2e
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # 4.2.2
- name: Set up Node.js ${{ matrix.node.version }}
uses: actions/setup-node@39370e3970a6d050c480ffad4ff0ed4d3fdee5af # 4.1.0
with:
node-version: ${{ matrix.node.version }}
cache: 'npm'
- run: npm ci
- run: |
node \
--run test \
--test-coverage-lines=0.8 \
--test-reporter-destination=./coverage.lcov \
--test-reporter=lcov
11 changes: 8 additions & 3 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
{
"editor.formatOnSave": true,
"editor.defaultFormatter": "biomejs.biome",
"javascript.updateImportsOnFileMove.enabled": "always",
"typescript.updateImportsOnFileMove.enabled": "always"
}
"typescript.updateImportsOnFileMove.enabled": "always",
"editor.formatOnPaste": true,
"editor.wordWrap": "wordWrapColumn",
"editor.wordWrapColumn": 100,
"[markdown]": {
"editor.wordWrap": "off"
}
}
14 changes: 7 additions & 7 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,18 @@ A recipe generally has a few things:
* A `README.md` explaining its purpose and use (including any options, and required and optional
files).
* Tests via node's test runner (min coverage: 80%)
* unit tests (file extension: `.spec.mjs` or `.spec.mts`)
* end-to-end test(s) for accepted use-cases (file extension: `.e2e.mjs` or `.e2e.mts`)
* unit tests
* end-to-end test(s) for accepted use-cases
* a `test` command in `package.json`; there may be sub-commands like `test:unit` & `test:e2e`, but there must be a parent that combines them.
* Include `--import='../../build/snapshots.mts` to standardise the filename (`${original_base_name}.snap.cjs`) across recipes.
* Ensure `--test-coverage-include` and `--test-coverage-exclude` are set correctly for the recipe's workspace. The root repo handles setting coverage rules like minimum line coverage.
* Code comments (js docs, etc)
* Types (either via typescript or jsdoc)

CI will run lint & type checking and all included test files against all PRs.

> [!NOTE]
> snapshots will be generated with the file extension `.snap.cjs`.

New recipes are added under `./recipes` in their own folder, succinctly named for what it does. General-purpose recipes have simple names like `correct-ts-specifiers`. A suite of migrations has a name like `migrate from 18 to 20`, and more specific migrations are named like `migrate fs.readFile from 18 to 20`.
New recipes are added under `./recipes` in their own folder, succinctly named for what they do. General-purpose recipes have simple names like `correct-ts-specifiers`. A suite of migrations has a name like `migrate from 18 to 20`, and more specific migrations are named like `migrate-fs-readFile-from-18-to-20`.

## Before pushing a commit

A convenient superset of checks is available via `node --run pre-commit`, which automatically fixes formatting and linting issues, checks types, and runs unit and end-to-end tests. Changes resulting from this should be committed.
A convenient superset of checks is available via `node --run pre-commit`, which automatically fixes formatting and linting issues (that are safe to fix), checks types, and runs tests. Changes resulting from this should be committed.
26 changes: 22 additions & 4 deletions biome.jsonc
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

How do we set options for use-block-statements? (I think we should use "multi")

Copy link
Member

Choose a reason for hiding this comment

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

(I think we should use "multi")

It's sad but I don't see support for an equivalent to multi. But I don't find it a problem personally, because I think it's fine to force the {...} block whatever the condition.

Original file line number Diff line number Diff line change
@@ -1,18 +1,31 @@
{
"$schema": "https://biomejs.dev/schemas/1.9.4/schema.json",
"formatter": {
"indentStyle": "tab",
"lineWidth": 100
"files": {
"ignore": [
"**/*.snap.cjs"
]
},
// "formatter": {
// "indentStyle": "tab",
// "lineWidth": 100,
// "useEditorconfig": true
// },
JakobJingleheimer marked this conversation as resolved.
Show resolved Hide resolved
"organizeImports": {
"enabled": false
},
// Rules for the linter
"linter": {
"rules": {
"style": {
"noNonNullAssertion": "off",
"noParameterAssign": "off",
"noYodaExpression": "error",
"useImportType": "error",
"useNodeAssertStrict": "error",
"useNodejsImportProtocol": "error"
},
"suspicious": {
"noAssignInExpressions": "off",
"noExplicitAny": "error",
"noEmptyBlock": "error",
"noDuplicateAtImportRules": "error",
Expand All @@ -25,12 +38,16 @@
},
"nursery": {
"noEnum": "error"
},
"performance": {
"recommended": true
}
}
},
// Language specific settings
"javascript": {
"formatter": {
"arrowParentheses": "always",
"semicolons": "always",
"quoteStyle": "single",
"trailingCommas": "all"
Expand All @@ -41,7 +58,8 @@
},
"json": {
"formatter": {
"enabled": false
"enabled": true,
"indentStyle": "space"
}
},
// VSC specific settings
Expand Down
9 changes: 3 additions & 6 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,11 @@
"version": "1.0.0",
"description": "A collection of migration recipes for userland code.",
"scripts": {
"format:fix": "biome format --fix ./",
"format": "biome format ./",
"lint:fix": "biome lint --fix ./",
"lint": "biome lint ./",
"pre-commit": "node --run lint:fix; node --run format:fix; node --run test:types; node --run test:unit; node --run test:e2e",
"test:e2e": "node --no-warnings --experimental-test-coverage --test-reporter=lcov --test-reporter-destination=./coverage.lcov --test-reporter=spec --test-reporter-destination=stdout --import './build/snapshots.ts' --test --test-coverage-include='recipes/**/*' --test-coverage-exclude='**/*.e2e.{mjs,mts}' './packages/*/*.e2e.{mjs,mts}'",
"test:types": "tsc",
"test:unit": "node --no-warnings --experimental-test-coverage --test-reporter=lcov --test-reporter-destination=./coverage.lcov --test-reporter=spec --test-reporter-destination=stdout --experimental-test-module-mocks --import './build/snapshots.ts' --test --test-coverage-include='recipes/**/*' --test-coverage-exclude='**/*.spec.{mjs,mts}' --test-coverage-lines=0.8 './packages/*/*.spec.{mjs,mts}'"
"pre-commit": "node --run lint:fix; node --run type-check; node --run test",
"test": "npm run test --workspaces -- --no-warnings --experimental-test-coverage --test-reporter=spec --test-reporter-destination=stdout",
"type-check": "tsc"
},
"repository": {
"type": "git",
Expand Down
29 changes: 26 additions & 3 deletions tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"rootDir": "./",

"target": "ESNext",
// "moduleDetection": "force",
"lib": ["ESNext"],

/* Modules */
"module": "NodeNext",
Expand All @@ -24,7 +24,7 @@

/* Interop Constraints */
"verbatimModuleSyntax": true,
"allowSyntheticDefaultImports": false,
"allowSyntheticDefaultImports": true,
JakobJingleheimer marked this conversation as resolved.
Show resolved Hide resolved
"esModuleInterop": true,
"forceConsistentCasingInFileNames": true,

Expand All @@ -40,5 +40,28 @@
"include": [
"./build/",
"./recipes/"
]
],
"exclude": [
"node_modules",
"**/build/**",
"**/fixtures/**",
"**/test/**",
"**/*.fixture.mjs",
"**/*.fixture.mts",
"**/*.fixture.js",
"**/*.fixture.ts",
"**/*.mock.mjs",
"**/*.e2e.mjs",
"**/*.e2e.mts",
"**/*.e2e.js",
"**/*.e2e.ts",
"**/*.spec.mjs",
"**/*.spec.mts",
"**/*.spec.js",
"**/*.spec.ts",
Comment on lines +49 to +61
Copy link
Member Author

Choose a reason for hiding this comment

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

This is super verbose, and the tsconfig docs specifically say they support full globs, but using {mjs,mts,js,ts} silently fails.

"**/*.test.mjs",
"**/*.test.mts",
"**/*.test.js",
"**/*.test.ts",
],
}
Loading