chore: improve tooling#19
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the repository’s developer tooling and applies consistent formatting across the TypeScript codebase, adding linting/formatting automation (ESLint flat config, oxfmt, lint-staged, git hooks) alongside associated doc/workflow updates.
Changes:
- Add ESLint (flat config) + oxfmt formatting scripts and config; wire up pre-commit via lint-staged.
- Reformat existing TS sources/tests to the new style (semicolons, quotes, wrapping).
- Minor docs/workflow quoting tweaks aligned with the tooling changes.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/transcript.ts | Formatting-only changes (semi/line wrapping). |
| src/notes-reader.ts | Formatting changes plus a modified BunShellLike type (currently introduces a TS error). |
| src/notes-reader.test.ts | Formatting-only changes. |
| src/index.ts | Formatting-only changes. |
| src/index.test.ts | Formatting-only changes. |
| package.json | Adds lint/format scripts + lint-staged hook and new devDependencies. |
| eslint.config.js | Introduces ESLint flat configuration for TS + prettier compatibility rules. |
| commitlint.config.ts | Formatting-only change (semicolon). |
| bun.lock | Lockfile updates for newly added tooling dependencies. |
| AGENTS.md | Documentation formatting updates (currently includes a rendering regression at the end). |
| .oxfmtrc.json | Adds oxfmt configuration. |
| .lintstagedrc.json | Adds lint-staged configuration. |
| .github/workflows/release.yml | Quote style update for workflow inputs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| > `Bun.$\`ls\``instead of execa. | ||
| Bun automatically loads`.env` — don't use dotenv. |
There was a problem hiding this comment.
The last two lines lost the blockquote marker and spacing around inline code, which breaks the rendered Markdown (e.g. `Bun.$\`ls\instead and loads.env ``). Please restore the intended > prefix and spaces so this section renders correctly.
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| (strings: TemplateStringsArray, ...values: any[]): { text(): Promise<string> } | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any, no-unused-vars | ||
| (strings: TemplateStringsArray, ...values: any): { text(): Promise<string> }; |
There was a problem hiding this comment.
BunShellLike's rest parameter is typed as ...values: any, but TypeScript requires rest parameters to be an array/tuple type (e.g. any[] / unknown[]). As written, this will fail typechecking and may also confuse ESLint/TS parser.
| (strings: TemplateStringsArray, ...values: any): { text(): Promise<string> }; | |
| (strings: TemplateStringsArray, ...values: any[]): { text(): Promise<string> }; |
mcmunder
force-pushed
the
chore/improve-tooling
branch
2 times, most recently
from
eebedc0 to
7579c23
Compare
mcmunder
force-pushed
the
chore/improve-tooling
branch
from
7579c23 to
d9de351
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| plugins: { | ||
| "@typescript-eslint": tseslint.plugin, | ||
| }, | ||
| rules: { | ||
| ...js.configs.recommended.rules, | ||
| ...tseslint.configs.recommended.rules, | ||
| ...prettier.rules, | ||
| "@typescript-eslint/explicit-function-return-type": "off", | ||
| "@typescript-eslint/no-explicit-any": "warn", | ||
| "no-console": "error", | ||
| }, |
There was a problem hiding this comment.
The new ESLint flat config appears to be applying recommended configs incorrectly by spreading .rules from typescript-eslint (and @eslint/js) instead of including the recommended config objects/arrays. In typescript-eslint v8, configs.recommended is a config array, so tseslint.configs.recommended.rules will be undefined and the recommended TS rules won’t be enabled. Consider using js.configs.recommended and ...tseslint.configs.recommended (plus prettier) as config entries, then override rules in a final object.
mcmunder
force-pushed
the
chore/improve-tooling
branch
from
d9de351 to
6cbdcd0
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // eslint-disable-next-line @typescript-eslint/no-explicit-any, no-unused-vars | ||
| (strings: TemplateStringsArray, ...values: any): { text(): Promise<string> } |
There was a problem hiding this comment.
BunShellLike defines a rest parameter as ...values: any, but TypeScript requires rest parameters to be an array/tuple type. This will fail type-checking; use any[] / unknown[] (and adjust the eslint-disable accordingly) so call sites like the tests’ ...values: unknown[] remain compatible.
| // eslint-disable-next-line @typescript-eslint/no-explicit-any, no-unused-vars | |
| (strings: TemplateStringsArray, ...values: any): { text(): Promise<string> } | |
| // eslint-disable-next-line no-unused-vars | |
| (strings: TemplateStringsArray, ...values: unknown[]): { text(): Promise<string> } |
| > `Bun.serve()` supports WebSockets, HTTPS, and routes — don't use Express. | ||
| > HTML imports with `Bun.serve()` for frontend — don't use Vite. | ||
| > `Bun.$\`ls\`` instead of execa. | ||
| > `Bun.$` instead of execa. |
There was a problem hiding this comment.
The guidance line `Bun.$` instead of execa. is ambiguous because Bun.$ is a template tag and needs a command template literal to be actionable. Consider updating this to a syntactically correct example (and format it so the backticks render correctly in Markdown), e.g. showing Bun.$\ls`` or similar.
| > `Bun.$` instead of execa. | |
| > Use Bun's process spawning helper (for example, `await Bun.$`git status`;`) instead of execa. |
No description provided.