Conversation
|
|
Caution Review failedThe pull request is closed. WalkthroughThis update introduces a type-level benchmarking workflow into the project. It adds new scripts and configuration for running TypeScript benchmarks, integrates these benchmarks into the continuous integration (CI) pipeline, and updates package management files to include necessary dependencies. Benchmark test files are added to measure the instantiation and transform chaining performance of a source adapter builder. TypeScript configuration is extended to include benchmark files, and workspace and build tool configurations are updated to support the new benchmarking tasks. Additionally, a minor change is made to the Vitest test setup file path. Changes
Sequence Diagram(s)sequenceDiagram
participant CI
participant Turbo
participant Benchmarks
participant TypeScript
CI->>Turbo: Run bench:types (via pnpm script)
Turbo->>Benchmarks: Execute .ts-benchmarks/index.ts
Benchmarks->>TypeScript: Compile and run benchmark tests
TypeScript-->>Benchmarks: Benchmark results
Benchmarks-->>Turbo: Report threshold status
Turbo-->>CI: Continue workflow based on benchmark outcome
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
vitest.config.tsOops! Something went wrong! :( ESLint: 9.23.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by packages/adapters/.ts-benchmarks/index.tsOops! Something went wrong! :( ESLint: 9.23.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by packages/adapters/.ts-benchmarks/builders/source-builder.ts-bench.tsOops! Something went wrong! :( ESLint: 9.23.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (9)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull Request Overview
This PR sets up @ark/attest for benchmarking typechecking performance. Key changes include updating configuration files to point to new setup files, adding the required dependencies for attest and tsx, introducing TypeScript benchmarks for the source adapter builder, and updating the CI workflow to run benchmarks.
Reviewed Changes
Copilot reviewed 6 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.config.ts | Updated msw file path from ./test/setup/msw.ts to ./test/global-setup/msw.ts |
| pnpm-workspace.yaml | Added new dependencies (@ark/attest and tsx) |
| packages/adapters/.ts-benchmarks/index.ts | Imported the new benchmark builder |
| packages/adapters/.ts-benchmarks/builders/source-builder.ts-bench.ts | Introduced benchmarking tests using @ark/attest and defined multiple transforms |
| .github/workflows/ci.yaml | Added a CI step to run benchmarks |
Files not reviewed (5)
- package.json: Language not supported
- packages/adapters/package.json: Language not supported
- packages/adapters/tsconfig.json: Language not supported
- pnpm-lock.yaml: Language not supported
- turbo.json: Language not supported
Comments suppressed due to low confidence (1)
packages/adapters/.ts-benchmarks/builders/source-builder.ts-bench.ts:83
- [nitpick] Multiple withTransform blocks in this bench use identical configuration logic (e.g. hardcoded 'v14' URL and 'generic' parser). Consider extracting this common logic into a reusable helper to improve readability and maintainability.
.withTransform(
luxass
changed the base branch from
refactor_remove_typecheck_configurations_and_related_tsconfig_files
to
graphite-base/142
…p for snapshot updates
luxass
force-pushed
the
feat_setup_attest_for_type_benchmarking
branch
from
03868c1 to
431242e
Compare
This PR is setting
@ark/attestup for us to improve our typechecking performance.Summary by CodeRabbit