-
Notifications
You must be signed in to change notification settings - Fork 439
ci(lint): ratchet as-any / @ts-ignore / eslint-disable counts #2658
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| { | ||
| "app": { | ||
| "as-any": 217, | ||
| "ts-ignore": 14, | ||
| "eslint-disable": 120 | ||
| }, | ||
| "api": { | ||
| "as-any": 97, | ||
| "ts-ignore": 4, | ||
| "eslint-disable": 26 | ||
| }, | ||
| "common-utils": { | ||
| "as-any": 69, | ||
| "ts-ignore": 5, | ||
| "eslint-disable": 32 | ||
| }, | ||
| "cli": { | ||
| "as-any": 0, | ||
| "ts-ignore": 0, | ||
| "eslint-disable": 3 | ||
| }, | ||
| "hdx-eval": { | ||
| "as-any": 0, | ||
| "ts-ignore": 0, | ||
| "eslint-disable": 1 | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| #!/usr/bin/env node | ||
| /** | ||
| * Ratchet: counts of tracked escape hatches may only go down. | ||
| * Above baseline -> fail (you added one; remove it). | ||
| * Below baseline -> warn only (run `yarn ratchet:update` to lock the | ||
| * improvement in). Non-fatal so an improvement can never turn `main` red — | ||
| * e.g. when two count-lowering PRs merge close together and `main`'s | ||
| * committed baseline briefly sits above the real count. | ||
| */ | ||
| import { execFileSync } from 'node:child_process'; | ||
| import { existsSync, readFileSync, writeFileSync } from 'node:fs'; | ||
| import { fileURLToPath } from 'node:url'; | ||
| import path from 'node:path'; | ||
|
|
||
| const ROOT = path.resolve( | ||
| path.dirname(fileURLToPath(import.meta.url)), | ||
| '../..', | ||
| ); | ||
| const BASELINE = path.join(ROOT, 'scripts/ci/ratchet-baseline.json'); | ||
| const PACKAGES = ['app', 'api', 'common-utils', 'cli', 'hdx-eval']; | ||
| const PATTERNS = { | ||
| 'as-any': 'as any', | ||
| 'ts-ignore': '@ts-ignore', | ||
| 'eslint-disable': 'eslint-disable', | ||
| }; | ||
|
Comment on lines
+23
to
+25
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Because Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Intentional — block |
||
|
|
||
| function count(pattern, dir) { | ||
| // A renamed/removed package (or a new PACKAGES entry added before its source | ||
| // exists) would make grep exit 2, not 1; treat a missing dir as zero rather | ||
| // than crashing the whole ratchet with an opaque stack trace. | ||
| if (!existsSync(dir)) return 0; | ||
| try { | ||
| const out = execFileSync( | ||
| 'grep', | ||
| ['-rEo', pattern, dir, '--include=*.ts', '--include=*.tsx'], | ||
| { encoding: 'utf8', maxBuffer: 64 * 1024 * 1024 }, | ||
| ); | ||
| return out.split('\n').filter(Boolean).length; | ||
| } catch (err) { | ||
| if (err.status === 1) return 0; // grep exit 1 = no matches | ||
| throw err; | ||
| } | ||
|
greptile-apps[bot] marked this conversation as resolved.
|
||
| } | ||
|
|
||
| const current = {}; | ||
| for (const pkg of PACKAGES) { | ||
| const dir = path.join(ROOT, 'packages', pkg, 'src'); | ||
| current[pkg] = Object.fromEntries( | ||
| Object.entries(PATTERNS).map(([name, re]) => [name, count(re, dir)]), | ||
| ); | ||
| } | ||
|
|
||
| if (process.argv.includes('--update')) { | ||
| writeFileSync(BASELINE, JSON.stringify(current, null, 2) + '\n'); | ||
| console.log(`ratchet baseline written to ${path.relative(ROOT, BASELINE)}`); | ||
| process.exit(0); | ||
| } | ||
|
|
||
| const baseline = JSON.parse(readFileSync(BASELINE, 'utf8')); | ||
| let failed = false; | ||
| for (const pkg of PACKAGES) { | ||
| for (const name of Object.keys(PATTERNS)) { | ||
| const now = current[pkg][name]; | ||
| const max = baseline[pkg]?.[name] ?? 0; | ||
| if (now > max) { | ||
| failed = true; | ||
| console.error( | ||
| `x ${pkg}/${name}: ${now} > baseline ${max} — remove the new occurrence(s)`, | ||
| ); | ||
| } else if (now < max) { | ||
| // Non-fatal: an improvement must never fail CI (it would red `main` and | ||
| // every open PR until someone re-baselines). Just nudge to lock it in. | ||
| console.warn( | ||
| `! ${pkg}/${name}: ${now} < baseline ${max} — run \`yarn ratchet:update\` to lock the improvement in`, | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| if (failed) process.exit(1); | ||
| console.log('ratchet ok: all escape-hatch counts at baseline'); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PACKAGESlists every tracked package by name. When a new package is added to the monorepo it will be silently skipped until someone remembers to update both this array andratchet-baseline.json. Auto-discovering packages from thepackages/directory (e.g. reading its subdirectories orpackage.jsonworkspaces) would make the ratchet self-updating, though it would also require that new packages always start with a baseline entry.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Intentional for now: the explicit
PACKAGESlist is mirrored 1:1 by the keys inratchet-baseline.json, so the two stay in lockstep and a new package is a deliberate, reviewed addition rather than silently folded in. Auto-discovery viapackages/*/srcis a reasonable follow-up but would need the baseline generation to track it too — out of scope for this PR.