Vasty improve linting performance - #146
Conversation
|
🌐 Preview URL: https://pr-146.frcsoftware.pages.dev |
github-actions
Bot
added
the
infra
Daniel1464
left a comment
Daniel1464
left a comment
There was a problem hiding this comment.
Great work! Here's another thing that probably should be bundled in this PR:
lint-staged actually passes the changed files as command-line args. Our lint-staged configuration calls pnpm format, which in turn calls prettier --write ., and pnpm lint, which calls eslint . and a bunch of other things. This meant that every time we made a commit, we were running the formatters on the whole repo instead of individual committed files.
To fix this, just change the lint-staged config in package.json to this:
"lint-staged": {
"**/*": [
"prettier --write",
"eslint",
"pnpm typecheck & pnpm lint:md & pnpm lint:regions",
"pnpm vale:setup && vale sync && vale"
],
"examples/**/*.java": "sh -c 'cd examples && ./gradlew spotlessApply'",
"src/data/glossary.ts": [
"pnpm generate:glossary",
"git add src/content/docs/resources/glossary.mdx"
]
}
This allows for prettier, eslint, and vale to run only on your committed files (the other lint scripts seem to fail if you do that.
|
See #140 |
|
Not ready to be merged, still integrating changes from #140 |
github-actions
Bot
added
the
Curriculum material
|
Ready for final review |
| @@ -1 +1,15 @@ | |||
| pnpm typecheck --minimumFailingSeverity warning | |||
| if git diff --cached --name-only | grep -q '\.astro$'; then | |||
There was a problem hiding this comment.
git diff --cached won't actually do anything, that checks differences between staged and latest commit, but for a pre-push hook everything will have already been committed
| "predev": "tsx scripts/generate-glossary.ts", | ||
| "lint": "pnpm lint:eslint && pnpm lint:md && pnpm lint:regions && pnpm lint:glossary && pnpm typecheck && pnpm vale", | ||
| "lint:eslint": "eslint .", | ||
| "lint:eslint": "eslint \"src/**/*.{ts,astro,js,mjs,css}\" \"*.mjs\" --cache", |
There was a problem hiding this comment.
We should lint scripts/ as well
| "typescript-eslint": "^8.61.1", | ||
| "vfile": "^6.0.3" | ||
| "vfile": "^6.0.3", | ||
| "typescript-7": "npm:typescript@^7.0.2" |
There was a problem hiding this comment.
remind me why ts 7 is being added?
There was a problem hiding this comment.
For tsc yes? I don't think ts 7 is actually being used with tsc, upon local checking.
There was a problem hiding this comment.
when we don't need to typecheck astro files, ts7 is ~6x faster (on my machine)
There was a problem hiding this comment.
For
tscyes? I don't think ts 7 is actually being used with tsc, upon local checking.
Works on my machine, maybe PATH issues? If you have a global typescript@6 that might be the issue.
| const m = line.match(CODE_REGION_SOURCE_ENTRY_RE); | ||
| if (m) { | ||
| sources.set(m[1]!, m[2]!); | ||
| extensions.add(m[2]!.split('.').at(-1)!); |
There was a problem hiding this comment.
Since we're only parsing extensions from the frontmatter, a full reference from a codeblock will get missed.
5 participants
Description
Fixed unintended analyzation of non-source files in the validate-regions script and set remark to only check for dead links in a CI environment.
Closes #143 and #87
Meta
Merge checklist: