Repository navigation
ci: make lint and format checks pass so they can be enforced - #850
Conversation
`npm run lint` exited 2 on every run: ESLint 9 no longer reads
.eslintrc.json, so it failed with "couldn't find an eslint.config file".
`prettier --check` failed on 190 files. CI hid both behind `|| echo`.
- Move the ESLint config to eslint.config.js (flat config) with the same
rules. Drop the jest env; tests use node:test and no-undef is off for
TypeScript. Add ignoreRestSiblings for `const { x, ...rest }` omits and
allow single-extends interfaces.
- Declare @eslint/js (already in the lockfile through eslint).
- Fix the 76 existing lint errors. No logic changes: unused imports,
constants and catch bindings removed, let -> const, one unused
parameter renamed to _options, an unused helper removed from index.ts,
and a disable comment on the intentional control-character regex.
- List the files that Prettier would currently reformat in
.prettierignore, so `prettier --check` passes without a mass reformat.
New files are checked. Remove entries as files get formatted.
`npm run lint` now exits 0 (warnings only) and `npm run format:check`
exits 0.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📜 Recent review details
📝 Summary
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 70.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 25 files. (3 skipped:… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description check The description clearly explains the ESLint migration, Prettier ignore changes, lint fixes, validation results, and the intended CI enforcement outcome. It is directly related to the changeset. Title check The title clearly and concisely summarizes the main change: making lint and format checks pass so CI enforcement can be enabled. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 70.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 25 files. (3 skipped: 2 unsupported, 1 too large.)
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
✨ Simplify code
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
1 participant
Summary
Both CI quality steps currently pass no matter what:
npm run lintexits 2 on every run. ESLint 9 no longer reads.eslintrc.json, so it stops with "couldn't find an eslint.config.(js|mjs|cjs) file".prettier --checkfails on 190 files. This PR makes both commands pass onmain, so the|| echofallbacks can be removed.No logic changes. Every source hunk is one of:
let→const;_options(utils/safe-redirect-fetch.ts);utils/url.ts).Changes:
eslint.config.js(flat config) replaces.eslintrc.json.jestenv is dropped, because the tests usenode:testandno-undefis off for TypeScript.@typescript-eslint/no-unused-varsgetsignoreRestSiblings, soconst { _confirmed, ...rest }-style omits pass.no-empty-object-typegetsallowInterfaces: "with-single-extends", which keepsSealedVerifyOptionsas is.package.json:lint/lint:fixbecomeeslint ./eslint . --fix, because--extis not supported by flat config on older ESLint 9 releases.@eslint/jsis now a declared devDependency. It is the same version already in the lockfile througheslint; the lockfile change is one line.no-unused-varsand 21prefer-const, as described above;isLoopbackBindHost()removed fromindex.ts: it lost its only caller in 3eab537;utils/url.tsis intentional and gets// eslint-disable-next-line no-control-regex..prettierignorelists the 190 files Prettier would reformat today, soprettier --checkpasses without a mass reformat that would conflict with open PRs./README.md), so nested files with the same name are still checked.The
|| echoremoval in.github/workflows/pr-test.ymlis not in this PR. The diff is below and can be applied by someone with workflow permissions:Related issue
None.
Tests
All run on Node 22.21.1:
npm run lint: exit 0. 0 errors, 997 warnings; warnings were already allowed.npm run format:check: exit 0. A newly added unformatted file is still reported.npm run build,npx tsc --noEmit,npm run check:runtime-deps: pass.npm run test:mock: 1492 passed, 0 failed (same count asmain).I ran the checks that cover this change.
Docs
README.md,README.ko.md,README.zh-CN.md, andREADME.pt-BR.mdmatch when user-facing behavior, env vars, flags, or examples changed. (No user-facing change.)docs/matches the same behavior.Breaking change
No. This changes dev tooling only; runtime behavior is unchanged.