optimize turbo and parallelize ci#124
Conversation
🦋 Changeset detectedLatest commit: 3659160 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughCI workflow refactored into phased execution model with consolidated commands. Multiple dependency versions updated across workspace packages. ESLint plugin rule metadata simplified by removing deprecated category fields. TypeScript JSX runtime switched to "react-jsx" in test app. CLI test refactored to use spawnSync. Turbo task configuration adjusted with narrowed inputs and added build dependencies. Changes
Possibly related PRs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
scripts/ci.js (2)
114-157: Consider adding a timeout for command execution.The
runCommandfunction has no timeout mechanism. If a command hangs (e.g., due to a deadlock or infinite loop), the CI script will wait indefinitely. Consider adding a timeout option tospawnor implementing manual timeout handling.🔎 Example timeout implementation
const runCommand = async (cmd) => { - const { command, name } = cmd; + const { command, name } = cmd; + const TIMEOUT_MS = 10 * 60 * 1000; // 10 minutes // Update status to running status[name].status = "running"; status[name].startTime = Date.now(); commandOutputs[name] = []; printStatus(); return new Promise((resolve) => { const proc = spawn(command, { shell: true, stdio: ["pipe", "pipe", "pipe"], }); + const timeout = setTimeout(() => { + proc.kill("SIGTERM"); + commandOutputs[name].push({ output: "Command timed out", type: "stderr" }); + }, TIMEOUT_MS); // ... existing handlers ... proc.on("close", (code) => { + clearTimeout(timeout); status[name].endTime = Date.now(); // ... }); }); };
59-67: Type safety consideration for status object.The
statusobject is initialized with an implicitanytype viareduce. While this works, adding JSDoc type annotations would improve maintainability and IDE support.🔎 Optional: Add JSDoc types
+/** + * @typedef {{ status: string; startTime: number; endTime: number; turboCache: boolean }} CommandStatus + * @type {Record<string, CommandStatus>} + */ const status = allCommands.reduce((acc, cmd) => { acc[cmd.name] = { endTime: 0, startTime: 0, status: "pending", turboCache: false, }; return acc; }, {});.github/workflows/ci.yaml (2)
59-79: Format commit may trigger recursive CI runs.When the format job commits and pushes changes, it will trigger a new
pull_request: synchronizeevent, potentially causing recursive CI runs. Consider either:
- Using
[skip ci]in the commit message- Using a bot token that doesn't trigger workflows
- Adding a condition to skip if the last commit was from the bot
🔎 Option: Add [skip ci] to commit message
if [[ -n "$(git status --porcelain)" ]]; then git add . - git commit -m "style: apply prettier formatting" + git commit -m "style: apply prettier formatting [skip ci]"Note:
[skip ci]would skip the entire workflow for the new commit. If you need partial runs, consider using a Personal Access Token that doesn't trigger workflows instead ofGITHUB_TOKEN.
26-32: Consider adding dependency caching to improve job performance.
oven-sh/setup-bun@v2only caches the Bun executable; project dependencies are not automatically cached. Each job independently runsbun install --frozen-lockfile, which downloads dependencies from scratch. Explicit caching withactions/cachewill significantly improve CI times, especially as the monorepo grows.🔎 Example: Add Bun cache
- name: Set up Bun uses: oven-sh/setup-bun@v2 with: bun-version-file: "package.json" - name: Cache Bun dependencies uses: actions/cache@v4 with: path: ~/.bun/install/cache key: ${{ runner.os }}-bun-${{ hashFiles('bun.lockb') }} restore-keys: | ${{ runner.os }}-bun- - name: Install dependencies run: bun install --frozen-lockfileAlso applies to: 48-54, 88-94, 108-114, 129-135, 148-154
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/ci.yamlscripts/ci.jsturbo.json
🧰 Additional context used
📓 Path-based instructions (1)
scripts/ci.js
📄 CodeRabbit inference engine (CLAUDE.md)
Use scripts/ci.js as the comprehensive CI entry script (format, lint, test, dependency checks, build)
Files:
scripts/ci.js
🧠 Learnings (6)
📓 Common learnings
Learnt from: drake-nathan
Repo: drake-nathan/js-style-kit PR: 84
File: scripts/ci.js:5-10
Timestamp: 2025-09-27T20:21:53.141Z
Learning: In scripts/ci.js for drake-nathan/js-style-kit: The build command must run before lint/format because the build process generates or modifies files that need to be linted/formatted. This script runs locally only and has specific build->lint dependencies.
Learnt from: CR
Repo: drake-nathan/js-style-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T02:53:12.206Z
Learning: Use Turborepo to manage the monorepo build pipeline and caching
Learnt from: CR
Repo: drake-nathan/js-style-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T02:53:12.206Z
Learning: Applies to scripts/ci.js : Use scripts/ci.js as the comprehensive CI entry script (format, lint, test, dependency checks, build)
📚 Learning: 2025-09-12T02:53:12.206Z
Learnt from: CR
Repo: drake-nathan/js-style-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T02:53:12.206Z
Learning: Use Turborepo to manage the monorepo build pipeline and caching
Applied to files:
.github/workflows/ci.yamlturbo.json
📚 Learning: 2025-09-12T02:53:12.206Z
Learnt from: CR
Repo: drake-nathan/js-style-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T02:53:12.206Z
Learning: Applies to scripts/ci.js : Use scripts/ci.js as the comprehensive CI entry script (format, lint, test, dependency checks, build)
Applied to files:
.github/workflows/ci.yamlscripts/ci.js
📚 Learning: 2025-09-12T02:53:12.206Z
Learnt from: CR
Repo: drake-nathan/js-style-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T02:53:12.206Z
Learning: Use Bun as the package manager and test runner across the repository
Applied to files:
.github/workflows/ci.yaml
📚 Learning: 2025-09-27T20:21:53.141Z
Learnt from: drake-nathan
Repo: drake-nathan/js-style-kit PR: 84
File: scripts/ci.js:5-10
Timestamp: 2025-09-27T20:21:53.141Z
Learning: In scripts/ci.js for drake-nathan/js-style-kit: The build command must run before lint/format because the build process generates or modifies files that need to be linted/formatted. This script runs locally only and has specific build->lint dependencies.
Applied to files:
.github/workflows/ci.yamlscripts/ci.jsturbo.json
📚 Learning: 2025-09-27T20:21:27.195Z
Learnt from: drake-nathan
Repo: drake-nathan/js-style-kit PR: 84
File: package.json:19-20
Timestamp: 2025-09-27T20:21:27.195Z
Learning: In the js-style-kit monorepo, the root lint script uses --ignore-pattern to exclude packages, apps, and docs because Turborepo runs individual lint scripts for each workspace. The root lint script is only meant to lint root-level files like scripts and config files, while `turbo run lint` handles workspace linting through the dependsOn: ["^lint"] configuration.
Applied to files:
.github/workflows/ci.yamlturbo.json
🧬 Code graph analysis (1)
scripts/ci.js (1)
packages/style-kit/bin/lib/setup-scripts.ts (1)
console(6-30)
🔇 Additional comments (11)
turbo.json (3)
3-3: Good improvement: centralizing lockfile as a global dependency.Moving
bun.locktoglobalDependenciescorrectly ensures all tasks invalidate when dependencies change, rather than duplicating this in individual task inputs.
5-7: Root-level tasks with empty configs will not be cached.Empty configurations for
//#lint,//#lint:fix, and//#sherifmean these root-level tasks won't benefit from Turborepo caching. If these tasks are executed viaturbo run, consider whether caching is intentionally disabled or if you want to addinputsfor cache validation.
17-21: Task dependencies correctly enforce build-first ordering.The
dependsOn: ["^build"]configuration ensures workspace builds complete before lint, test, and typecheck tasks run. This aligns with the established requirement that build generates files needed by these tasks. Based on learnings, this is the correct approach.Also applies to: 22-26, 32-35, 37-40, 42-46
scripts/ci.js (3)
4-28: Phase structure correctly separates build from dependent tasks.The two-phase model appropriately runs
buildin Phase 1, thentypecheck,lint, andtestin Phase 2 after build completes. This aligns with the learning that build generates files needed by lint.One consideration:
format(Prettier) running in parallel withbuildin Phase 1 should be fine since Prettier formats source files, not build outputs. However, if any generated files need formatting, you may want to verify this works as expected.
159-179: LGTM!The
runPhasefunction cleanly runs all phase commands in parallel and only displays output for failed commands, reducing noise on success. UsingPromise.allensures all commands complete before evaluating results.
181-219: LGTM!The sequential phase execution with early termination on failure is appropriate—there's no value in running Phase 2 validation if Phase 1 (including build) fails. The summary output and exit code handling are correct.
.github/workflows/ci.yaml (5)
13-15: Good practice: Turbo remote caching at workflow level.Setting
TURBO_TOKENandTURBO_TEAMas workflow-level environment variables enables Turborepo remote caching across all jobs, which is essential for cache sharing in the parallelized architecture.
99-117: Jobs depend onbuildbut don't receive build artifacts.The
typecheck,lint, andtestjobs declareneeds: [build]for ordering, but thebuildjob doesn't upload artifacts. These dependent jobs rely entirely on Turbo remote caching to avoid rebuilding. This works if remote caching is properly configured, but if the cache misses, each job will rebuild independently.Consider whether this is intentional or if you want to upload/download build artifacts for guaranteed consistency.
119-138: Lint job correctly depends on format and uses dynamic ref.The
needs: [build, format]ensures lint runs after formatting is complete. The checkout withref: ${{ github.head_ref || github.ref_name }}picks up any committed formatting changes. This is the correct approach.
140-163: LGTM!The test job correctly depends on build, runs coverage, and uploads to Codecov with
if: always()to ensure coverage reports are uploaded even on test failures.
18-35: LGTM!The sherif job correctly runs independently as it only checks package.json consistency and doesn't require build outputs.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
scripts/ci.js (1)
120-152: Consider handling spawn errors explicitly.If the command itself fails to spawn (e.g., command not found, permission denied), the
proc.on('error')event is emitted but not handled here. This could leave the promise hanging or cause unhandled exceptions.🔎 Optional: Add error handler for spawn failures
proc.on("close", (code) => { status[name].endTime = Date.now(); status[name].status = code === 0 ? "success" : "failed"; printStatus(); resolve(code === 0); }); + + proc.on("error", (err) => { + commandOutputs[name].push({ + output: `Spawn error: ${err.message}`, + type: "stderr", + }); + status[name].endTime = Date.now(); + status[name].status = "failed"; + printStatus(); + resolve(false); + }); });.github/workflows/ci.yaml (1)
13-15: Configure Turbo remote cache secrets for optimal CI performance.The workflow uses
TURBO_TOKENandTURBO_TEAMto enable remote caching across jobs. Without these GitHub repository secrets/variables configured, each job will lack a remote cache and perform full rebuilds, significantly increasing CI runtime. Ensure these are set up in your repository settings.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/ci.yamlscripts/ci.js
🧰 Additional context used
📓 Path-based instructions (1)
scripts/ci.js
📄 CodeRabbit inference engine (CLAUDE.md)
Use scripts/ci.js as the comprehensive CI entry script (format, lint, test, dependency checks, build)
Files:
scripts/ci.js
🧠 Learnings (6)
📓 Common learnings
Learnt from: drake-nathan
Repo: drake-nathan/js-style-kit PR: 84
File: scripts/ci.js:5-10
Timestamp: 2025-09-27T20:21:53.141Z
Learning: In scripts/ci.js for drake-nathan/js-style-kit: The build command must run before lint/format because the build process generates or modifies files that need to be linted/formatted. This script runs locally only and has specific build->lint dependencies.
Learnt from: CR
Repo: drake-nathan/js-style-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T02:53:12.206Z
Learning: Applies to scripts/ci.js : Use scripts/ci.js as the comprehensive CI entry script (format, lint, test, dependency checks, build)
📚 Learning: 2025-09-12T02:53:12.206Z
Learnt from: CR
Repo: drake-nathan/js-style-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T02:53:12.206Z
Learning: Use Turborepo to manage the monorepo build pipeline and caching
Applied to files:
.github/workflows/ci.yaml
📚 Learning: 2025-09-12T02:53:12.206Z
Learnt from: CR
Repo: drake-nathan/js-style-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T02:53:12.206Z
Learning: Use Bun as the package manager and test runner across the repository
Applied to files:
.github/workflows/ci.yaml
📚 Learning: 2025-09-27T20:21:53.141Z
Learnt from: drake-nathan
Repo: drake-nathan/js-style-kit PR: 84
File: scripts/ci.js:5-10
Timestamp: 2025-09-27T20:21:53.141Z
Learning: In scripts/ci.js for drake-nathan/js-style-kit: The build command must run before lint/format because the build process generates or modifies files that need to be linted/formatted. This script runs locally only and has specific build->lint dependencies.
Applied to files:
.github/workflows/ci.yamlscripts/ci.js
📚 Learning: 2025-09-27T20:21:27.195Z
Learnt from: drake-nathan
Repo: drake-nathan/js-style-kit PR: 84
File: package.json:19-20
Timestamp: 2025-09-27T20:21:27.195Z
Learning: In the js-style-kit monorepo, the root lint script uses --ignore-pattern to exclude packages, apps, and docs because Turborepo runs individual lint scripts for each workspace. The root lint script is only meant to lint root-level files like scripts and config files, while `turbo run lint` handles workspace linting through the dependsOn: ["^lint"] configuration.
Applied to files:
.github/workflows/ci.yaml
📚 Learning: 2025-09-12T02:53:12.206Z
Learnt from: CR
Repo: drake-nathan/js-style-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T02:53:12.206Z
Learning: Applies to scripts/ci.js : Use scripts/ci.js as the comprehensive CI entry script (format, lint, test, dependency checks, build)
Applied to files:
.github/workflows/ci.yamlscripts/ci.js
🔇 Additional comments (11)
scripts/ci.js (4)
4-24: Phase structure correctly implements build dependencies.The two-phase design properly ensures that
buildcompletes beforeformat,lint,typecheck, andtestrun—aligning with the requirement that build generates files needed by linting/formatting. Based on learnings, this dependency ordering is correct.
66-105: Status rendering is well-implemented.The phase-aware status display with dimmed headers, per-command coloring, and turbo cache hit indicators provides clear visual feedback during CI execution.
177-215: LGTM!The execution flow correctly runs phases sequentially, fails fast on errors, and provides a clear final summary with total duration. The fail-fast approach is appropriate for CI where early feedback is valuable.
217-221: LGTM!The interrupt handler properly catches SIGINT and exits with a non-zero code, allowing clean termination.
.github/workflows/ci.yaml (7)
13-15: Good practice: Turbo remote caching environment variables at workflow level.Setting
TURBO_TOKENandTURBO_TEAMat the top level ensures all jobs can leverage remote caching, which is essential for sharing build artifacts across the separate jobs.
18-35: LGTM!The sherif job runs independently without dependencies, correctly implementing parallel Phase 1 execution alongside the build job.
85-101: LGTM!The build job is correctly isolated and will populate the Turbo remote cache, enabling fast cache hits in dependent jobs.
123-142: Lint job correctly depends on both build and format.The dependency chain ensures:
- Build completes (populating cache)
- Format commits any changes
- Lint runs on the formatted code with
--max-warnings 0for strict checkingBased on learnings, the build→lint dependency is correctly enforced.
144-167: LGTM!The test job correctly depends on build, runs coverage, and uploads to Codecov with
if: always()to capture coverage even if tests fail.
103-121: The typecheck job correctly declares build as a dependency.The typecheck job has
needs: [build]just like the format job, and additionally benefits from Turbo's dependency management viaturbo.json, wheretypecheckdeclaresdependsOn: ["^build"]. Both mechanisms ensure build outputs are available before typecheck runs.Likely an incorrect or invalid review comment.
37-58: Remove the redundant build step; Turbo remote caching is not configured.The format job re-runs
bunx turbo run buildexpecting a cache hit, but this will not occur because:
- Turbo remote cache is not configured in
turbo.json- GitHub Actions jobs run on isolated runners with no shared filesystem
- The
needs: [build]dependency only ensures job ordering, not artifact/cache sharingThis rebuild is wasteful. Either remove the build step if not needed for prettier, or configure Turbo remote caching properly to enable cache reuse across isolated runners.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/ci.yaml (2)
31-36: Consider adding Turbo cache for faster builds.While Turbo remote caching is configured, adding a local cache layer for the
.turbodirectory would significantly speed up cache restoration. Local cache is faster than fetching from remote cache.🔎 Proposed enhancement to add Turbo cache
Add this step after the Bun dependency cache in each job:
- name: Cache Bun dependencies uses: actions/cache@v4 with: path: ~/.bun/install/cache key: bun-${{ runner.os }}-${{ hashFiles('bun.lock') }} restore-keys: bun-${{ runner.os }}- + + - name: Cache Turbo + uses: actions/cache@v4 + with: + path: .turbo + key: turbo-${{ runner.os }}-${{ github.sha }} + restore-keys: turbo-${{ runner.os }}-Apply this pattern to all jobs (sherif, build, format, typecheck, lint, test) for consistent performance improvement.
Also applies to: 56-61
3-6: Consider scopingcontents: writepermission to format job only.Currently, all jobs inherit
contents: write, but only the format job needs it to push formatting changes. Following the principle of least privilege, you could scope this permission to just the format job.🔎 Proposed permission scoping
Move
contents: writefrom workflow level to the format job:permissions: checks: write pull-requests: write - contents: write # ... rest of workflow ... format: name: Format needs: [build] runs-on: ubuntu-latest + permissions: + contents: write steps:Other jobs would inherit read-only access by default, improving security posture.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yaml
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: drake-nathan
Repo: drake-nathan/js-style-kit PR: 84
File: scripts/ci.js:5-10
Timestamp: 2025-09-27T20:21:53.141Z
Learning: In scripts/ci.js for drake-nathan/js-style-kit: The build command must run before lint/format because the build process generates or modifies files that need to be linted/formatted. This script runs locally only and has specific build->lint dependencies.
Learnt from: CR
Repo: drake-nathan/js-style-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T02:53:12.206Z
Learning: Applies to scripts/ci.js : Use scripts/ci.js as the comprehensive CI entry script (format, lint, test, dependency checks, build)
Learnt from: drake-nathan
Repo: drake-nathan/js-style-kit PR: 84
File: package.json:19-20
Timestamp: 2025-09-27T20:21:27.195Z
Learning: In the js-style-kit monorepo, the root lint script uses --ignore-pattern to exclude packages, apps, and docs because Turborepo runs individual lint scripts for each workspace. The root lint script is only meant to lint root-level files like scripts and config files, while `turbo run lint` handles workspace linting through the dependsOn: ["^lint"] configuration.
Learnt from: CR
Repo: drake-nathan/js-style-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T02:53:12.206Z
Learning: Use Turborepo to manage the monorepo build pipeline and caching
📚 Learning: 2025-09-12T02:53:12.206Z
Learnt from: CR
Repo: drake-nathan/js-style-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T02:53:12.206Z
Learning: Use Turborepo to manage the monorepo build pipeline and caching
Applied to files:
.github/workflows/ci.yaml
📚 Learning: 2025-09-12T02:53:12.206Z
Learnt from: CR
Repo: drake-nathan/js-style-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T02:53:12.206Z
Learning: Use Bun as the package manager and test runner across the repository
Applied to files:
.github/workflows/ci.yaml
📚 Learning: 2025-09-27T20:21:53.141Z
Learnt from: drake-nathan
Repo: drake-nathan/js-style-kit PR: 84
File: scripts/ci.js:5-10
Timestamp: 2025-09-27T20:21:53.141Z
Learning: In scripts/ci.js for drake-nathan/js-style-kit: The build command must run before lint/format because the build process generates or modifies files that need to be linted/formatted. This script runs locally only and has specific build->lint dependencies.
Applied to files:
.github/workflows/ci.yaml
📚 Learning: 2025-09-27T20:21:27.195Z
Learnt from: drake-nathan
Repo: drake-nathan/js-style-kit PR: 84
File: package.json:19-20
Timestamp: 2025-09-27T20:21:27.195Z
Learning: In the js-style-kit monorepo, the root lint script uses --ignore-pattern to exclude packages, apps, and docs because Turborepo runs individual lint scripts for each workspace. The root lint script is only meant to lint root-level files like scripts and config files, while `turbo run lint` handles workspace linting through the dependsOn: ["^lint"] configuration.
Applied to files:
.github/workflows/ci.yaml
📚 Learning: 2025-09-12T02:53:12.206Z
Learnt from: CR
Repo: drake-nathan/js-style-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T02:53:12.206Z
Learning: Applies to scripts/ci.js : Use scripts/ci.js as the comprehensive CI entry script (format, lint, test, dependency checks, build)
Applied to files:
.github/workflows/ci.yaml
🔇 Additional comments (4)
.github/workflows/ci.yaml (4)
13-15: LGTM: Turbo remote caching configured correctly.Environment variables are properly sourced from secrets and vars, enabling Turbo's remote caching across all jobs.
70-98: Format job structure is correct.The dependency on build and the rebuild step for prettier config align with the project's established pattern where build generates files needed for formatting.
Based on learnings, the build command must run before format.
156-180: Lint dependencies correctly ordered.The dual dependency on both
buildandformatensures that:
- ESLint config is available from the build step
- Linting runs on the formatted code after format pushes changes
The checkout with
ref: ${{ github.head_ref || github.ref_name }}correctly fetches the potentially formatted code.
125-152: Typecheck and Test jobs properly depend on build.Both jobs correctly:
- Depend on the build job to ensure type definitions and test imports are available
- Rebuild using Turbo (which should hit cache for fast execution)
- Follow the established pattern for Phase 2 validation
Also applies to: 185-209
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yaml
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: drake-nathan
Repo: drake-nathan/js-style-kit PR: 84
File: scripts/ci.js:5-10
Timestamp: 2025-09-27T20:21:53.141Z
Learning: In scripts/ci.js for drake-nathan/js-style-kit: The build command must run before lint/format because the build process generates or modifies files that need to be linted/formatted. This script runs locally only and has specific build->lint dependencies.
Learnt from: CR
Repo: drake-nathan/js-style-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T02:53:12.206Z
Learning: Applies to scripts/ci.js : Use scripts/ci.js as the comprehensive CI entry script (format, lint, test, dependency checks, build)
Learnt from: drake-nathan
Repo: drake-nathan/js-style-kit PR: 84
File: package.json:19-20
Timestamp: 2025-09-27T20:21:27.195Z
Learning: In the js-style-kit monorepo, the root lint script uses --ignore-pattern to exclude packages, apps, and docs because Turborepo runs individual lint scripts for each workspace. The root lint script is only meant to lint root-level files like scripts and config files, while `turbo run lint` handles workspace linting through the dependsOn: ["^lint"] configuration.
📚 Learning: 2025-09-12T02:53:12.206Z
Learnt from: CR
Repo: drake-nathan/js-style-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T02:53:12.206Z
Learning: Use Turborepo to manage the monorepo build pipeline and caching
Applied to files:
.github/workflows/ci.yaml
📚 Learning: 2025-09-12T02:53:12.206Z
Learnt from: CR
Repo: drake-nathan/js-style-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T02:53:12.206Z
Learning: Applies to scripts/ci.js : Use scripts/ci.js as the comprehensive CI entry script (format, lint, test, dependency checks, build)
Applied to files:
.github/workflows/ci.yaml
📚 Learning: 2025-09-12T02:53:12.206Z
Learnt from: CR
Repo: drake-nathan/js-style-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T02:53:12.206Z
Learning: Use Bun as the package manager and test runner across the repository
Applied to files:
.github/workflows/ci.yaml
📚 Learning: 2025-09-27T20:21:53.141Z
Learnt from: drake-nathan
Repo: drake-nathan/js-style-kit PR: 84
File: scripts/ci.js:5-10
Timestamp: 2025-09-27T20:21:53.141Z
Learning: In scripts/ci.js for drake-nathan/js-style-kit: The build command must run before lint/format because the build process generates or modifies files that need to be linted/formatted. This script runs locally only and has specific build->lint dependencies.
Applied to files:
.github/workflows/ci.yaml
📚 Learning: 2025-09-27T20:21:27.195Z
Learnt from: drake-nathan
Repo: drake-nathan/js-style-kit PR: 84
File: package.json:19-20
Timestamp: 2025-09-27T20:21:27.195Z
Learning: In the js-style-kit monorepo, the root lint script uses --ignore-pattern to exclude packages, apps, and docs because Turborepo runs individual lint scripts for each workspace. The root lint script is only meant to lint root-level files like scripts and config files, while `turbo run lint` handles workspace linting through the dependsOn: ["^lint"] configuration.
Applied to files:
.github/workflows/ci.yaml
🪛 actionlint (1.7.9)
.github/workflows/ci.yaml
53-53: "github.head_ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
(expression)
🔇 Additional comments (5)
.github/workflows/ci.yaml (5)
13-15: LGTM! Workflow-level environment variables simplify configuration.Defining
TURBO_TOKENandTURBO_TEAMat the workflow level is a good practice that eliminates duplication across jobs.
44-46: LGTM! Phase 1 parallelization is appropriate.Combining
buildandsherifallows Turbo to parallelize these independent tasks, while correctly maintaining the build-before-lint/format sequence (format is Phase 2, lint is Phase 3).
48-50: LGTM! Format step is correctly sequenced.Running format after build and before lint ensures that linting validates the formatted code, maintaining the proper build → format → lint dependency chain.
72-74: LGTM! Phase 3 consolidation is well-structured.Combining
typecheck,lint, andtest:coverageallows Turbo to parallelize these independent validation tasks while maintaining proper dependencies on build and format completion.
34-39: The cache path~/.bun/install/cacheis correct according to Bun's official documentation and matches the standard location for Bun's global install cache.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @apps/next-test-app/next-env.d.ts:
- Line 3: Remove the manual import of "./.next/dev/types/routes.d.ts" from
next-env.d.ts because it contradicts the file's "do not edit" guidance and
causes TS errors in fresh checkouts; delete the import statement (the line
containing import "./.next/dev/types/routes.d.ts";) and rely on Next.js's
automatic route type generation during next dev/build/typegen, leaving the rest
of next-env.d.ts unchanged.
In @packages/style-kit/bin/cli.test.ts:
- Around line 45-91: Replace the process.chdir/execSync pattern in the "detects
different package managers correctly" test with a child_process.spawnSync call
that sets cwd to testDir and captures stdout/stderr; specifically, stop mutating
global process.cwd(), remove the try/finally restore logic, and call spawnSync
like spawnSync("node", [CLI_PATH, "init"], { cwd: testDir, encoding: "utf8" })
(or equivalent) then assert against result.stdout || result.stderr to verify
"Using package manager: bun"; keep the existing test setup (package.json,
bun.lock), timeout, and variable names (CLI_PATH, testDir) so the change is
minimal.
In @packages/style-kit/package.json:
- Line 53: The package.json upgrade bumps for eslint-plugin-jsdoc and globals
introduce breaking changes; revert or pin "eslint-plugin-jsdoc" to the last
compatible 61.x (e.g., 61.5.0) and "globals" to 16.x (e.g., 16.5.0) in
packages/style-kit's package.json, run npm/yarn install to update the lockfile,
run the test suite and any consumer compatibility checks, and add a comment or
changelog entry noting Node engine requirements if you later decide to accept
62.0.0 (or alternatively add an "engines" field to package.json to require Node
>=20.19.0 if you intentionally move to eslint-plugin-jsdoc@62).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
.changeset/update-eslint-plugin-nextjs.md.changeset/update-style-kit-deps.md.github/workflows/ci.yamlapps/next-test-app/next-env.d.tsapps/next-test-app/package.jsonapps/next-test-app/tsconfig.jsonapps/vite-test-app/package.jsonpackage.jsonpackages/eslint-plugin-nextjs/package.jsonpackages/eslint-plugin-nextjs/src/rules/no-head-element.tspackages/eslint-plugin-nextjs/src/rules/no-html-link-for-pages.tspackages/eslint-plugin-nextjs/src/rules/no-img-element.tspackages/style-kit/bin/cli.test.tspackages/style-kit/package.json
💤 Files with no reviewable changes (3)
- packages/eslint-plugin-nextjs/src/rules/no-head-element.ts
- packages/eslint-plugin-nextjs/src/rules/no-html-link-for-pages.ts
- packages/eslint-plugin-nextjs/src/rules/no-img-element.ts
✅ Files skipped from review due to trivial changes (1)
- .changeset/update-style-kit-deps.md
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci.yaml
🧰 Additional context used
📓 Path-based instructions (1)
apps/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain integration test applications under the apps/ directory to validate real-world configurations
Files:
apps/next-test-app/package.jsonapps/vite-test-app/package.jsonapps/next-test-app/tsconfig.jsonapps/next-test-app/next-env.d.ts
🧠 Learnings (16)
📓 Common learnings
Learnt from: drake-nathan
Repo: drake-nathan/js-style-kit PR: 84
File: scripts/ci.js:5-10
Timestamp: 2025-09-27T20:21:53.141Z
Learning: In scripts/ci.js for drake-nathan/js-style-kit: The build command must run before lint/format because the build process generates or modifies files that need to be linted/formatted. This script runs locally only and has specific build->lint dependencies.
Learnt from: CR
Repo: drake-nathan/js-style-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T02:53:12.206Z
Learning: Applies to scripts/ci.js : Use scripts/ci.js as the comprehensive CI entry script (format, lint, test, dependency checks, build)
Learnt from: drake-nathan
Repo: drake-nathan/js-style-kit PR: 84
File: package.json:19-20
Timestamp: 2025-09-27T20:21:27.195Z
Learning: In the js-style-kit monorepo, the root lint script uses --ignore-pattern to exclude packages, apps, and docs because Turborepo runs individual lint scripts for each workspace. The root lint script is only meant to lint root-level files like scripts and config files, while `turbo run lint` handles workspace linting through the dependsOn: ["^lint"] configuration.
📚 Learning: 2025-10-16T22:50:32.349Z
Learnt from: drake-nathan
Repo: drake-nathan/js-style-kit PR: 96
File: packages/style-kit/src/eslint/convex/config.ts:0-0
Timestamp: 2025-10-16T22:50:32.349Z
Learning: For Convex ESLint configuration in packages/style-kit/src/eslint/convex/config.ts, the file pattern should only include `.js` and `.ts` files (not `.jsx` or `.tsx`), as Convex directories contain backend/server-side code and should not have React component files.
Applied to files:
apps/next-test-app/package.jsonapps/next-test-app/tsconfig.jsonpackages/style-kit/package.json
📚 Learning: 2025-09-12T02:53:12.206Z
Learnt from: CR
Repo: drake-nathan/js-style-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T02:53:12.206Z
Learning: Applies to packages/eslint-plugin-nextjs/test/** : Keep comprehensive tests for the Next.js ESLint plugin under packages/eslint-plugin-nextjs/test/
Applied to files:
packages/eslint-plugin-nextjs/package.jsonpackages/style-kit/bin/cli.test.tsapps/next-test-app/tsconfig.jsonpackages/style-kit/package.json
📚 Learning: 2025-09-12T02:53:12.206Z
Learnt from: CR
Repo: drake-nathan/js-style-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T02:53:12.206Z
Learning: Applies to packages/eslint-plugin-nextjs/src/rules/** : Place individual ESLint rule implementations for the Next.js plugin in packages/eslint-plugin-nextjs/src/rules/
Applied to files:
packages/eslint-plugin-nextjs/package.json.changeset/update-eslint-plugin-nextjs.md
📚 Learning: 2025-09-12T02:53:12.206Z
Learnt from: CR
Repo: drake-nathan/js-style-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T02:53:12.206Z
Learning: Applies to packages/eslint-plugin-nextjs/src/utils/** : Store shared utilities for ESLint rules under packages/eslint-plugin-nextjs/src/utils/
Applied to files:
packages/eslint-plugin-nextjs/package.json.changeset/update-eslint-plugin-nextjs.md
📚 Learning: 2025-03-15T03:32:01.610Z
Learnt from: drake-nathan
Repo: drake-nathan/js-style-kit PR: 39
File: packages/style-kit/package.json:65-65
Timestamp: 2025-03-15T03:32:01.610Z
Learning: The js-style-kit package continues to ship and support the Vitest ESLint plugin (`eslint-plugin-vitest`) as a dependency, even though the package itself has migrated from using Vitest to Bun for its internal testing.
Applied to files:
packages/eslint-plugin-nextjs/package.jsonapps/vite-test-app/package.jsonpackages/style-kit/bin/cli.test.tspackages/style-kit/package.json
📚 Learning: 2025-09-12T02:53:12.206Z
Learnt from: CR
Repo: drake-nathan/js-style-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T02:53:12.206Z
Learning: Applies to packages/style-kit/src/eslint/index.ts : Implement the main ESLint configuration factory at packages/style-kit/src/eslint/index.ts
Applied to files:
packages/eslint-plugin-nextjs/package.jsonpackages/style-kit/package.json
📚 Learning: 2025-09-12T02:53:12.206Z
Learnt from: CR
Repo: drake-nathan/js-style-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T02:53:12.206Z
Learning: Use tsup as the TypeScript bundler for packages
Applied to files:
packages/eslint-plugin-nextjs/package.jsonpackage.jsonapps/next-test-app/tsconfig.json
📚 Learning: 2025-09-27T20:21:27.195Z
Learnt from: drake-nathan
Repo: drake-nathan/js-style-kit PR: 84
File: package.json:19-20
Timestamp: 2025-09-27T20:21:27.195Z
Learning: In the js-style-kit monorepo, the root lint script uses --ignore-pattern to exclude packages, apps, and docs because Turborepo runs individual lint scripts for each workspace. The root lint script is only meant to lint root-level files like scripts and config files, while `turbo run lint` handles workspace linting through the dependsOn: ["^lint"] configuration.
Applied to files:
packages/eslint-plugin-nextjs/package.jsonpackage.jsonapps/next-test-app/tsconfig.json
📚 Learning: 2025-09-12T02:53:12.206Z
Learnt from: CR
Repo: drake-nathan/js-style-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T02:53:12.206Z
Learning: Use Bun as the package manager and test runner across the repository
Applied to files:
package.jsonpackages/style-kit/bin/cli.test.ts
📚 Learning: 2025-09-12T02:53:12.206Z
Learnt from: CR
Repo: drake-nathan/js-style-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T02:53:12.206Z
Learning: Use Turborepo to manage the monorepo build pipeline and caching
Applied to files:
package.json
📚 Learning: 2025-09-12T02:53:12.206Z
Learnt from: CR
Repo: drake-nathan/js-style-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T02:53:12.206Z
Learning: Applies to packages/style-kit/bin/index.ts : Provide the CLI entry point at packages/style-kit/bin/index.ts
Applied to files:
packages/style-kit/bin/cli.test.tspackages/style-kit/package.json
📚 Learning: 2025-09-27T20:21:53.141Z
Learnt from: drake-nathan
Repo: drake-nathan/js-style-kit PR: 84
File: scripts/ci.js:5-10
Timestamp: 2025-09-27T20:21:53.141Z
Learning: In scripts/ci.js for drake-nathan/js-style-kit: The build command must run before lint/format because the build process generates or modifies files that need to be linted/formatted. This script runs locally only and has specific build->lint dependencies.
Applied to files:
packages/style-kit/bin/cli.test.ts
📚 Learning: 2025-09-12T02:53:12.206Z
Learnt from: CR
Repo: drake-nathan/js-style-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T02:53:12.206Z
Learning: Applies to packages/style-kit/src/index.ts : Expose the main style-kit entry point from packages/style-kit/src/index.ts
Applied to files:
packages/style-kit/bin/cli.test.tspackages/style-kit/package.json
📚 Learning: 2025-09-12T02:53:12.206Z
Learnt from: CR
Repo: drake-nathan/js-style-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T02:53:12.206Z
Learning: Applies to packages/style-kit/src/eslint/** : Organize ESLint configuration modules under packages/style-kit/src/eslint/, with plugin-specific subfolders (e.g., base/, import/, react/)
Applied to files:
packages/style-kit/package.json
📚 Learning: 2025-09-12T02:53:12.206Z
Learnt from: CR
Repo: drake-nathan/js-style-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T02:53:12.206Z
Learning: Applies to packages/style-kit/src/prettier/** : Keep Prettier configuration (and its plugin support) under packages/style-kit/src/prettier/
Applied to files:
packages/style-kit/package.json
🧬 Code graph analysis (1)
packages/style-kit/bin/cli.test.ts (1)
packages/style-kit/bin/lib/detect-package-manager.test.ts (7)
mockFs(35-45)mockFs(47-55)mockFs(139-148)mockFs(25-33)mockFs(100-109)beforeEach(11-173)mockFs(111-122)
🔇 Additional comments (7)
.changeset/update-eslint-plugin-nextjs.md (1)
1-5: Changeset entry is properly formatted, but verify alignment with PR objectives.The .changeset file is syntactically correct and the patch bump is appropriate for a compatibility fix. However, the PR objectives describe "optimize turbo and parallelize ci," while this file documents an ESLint plugin compatibility update—these appear misaligned. The AI-generated summary mentions related updates to rule metadata, but those implementation files are not included in this review.
Please confirm that all relevant files from this PR have been provided for review (e.g., workflow files,
turbo.json,scripts/ci.js, or ESLint rule files mentioned in the summary).apps/next-test-app/tsconfig.json (2)
26-32: The.nextdirectory includes intsconfig.jsonare valid in this context.While
.next/types/**/*.tsand.next/dev/types/**/*.tsare generated files that don't exist until the build runs, the CI workflow ensures this is not a problem: the build step (Phase 1) completes before the typecheck step (Phase 3). Additionally,next-env.d.tsexplicitly imports from./.next/dev/types/routes.d.ts, so the tsconfig includes are consistent with Next.js's own type references. This configuration is appropriate for the integration test app.
7-7: Clarify why"react-jsx"is used instead of Next.js-recommended"preserve".Next.js officially recommends setting
"jsx": "preserve"to let the Next.js compiler (SWC) handle JSX transformation. This test app uses"react-jsx"instead, which is a valid alternative but non-standard. If this is intentional for testing alternative configurations, document the reason.⛔ Skipped due to learnings
Learnt from: drake-nathan Repo: drake-nathan/js-style-kit PR: 96 File: packages/style-kit/src/eslint/convex/config.ts:0-0 Timestamp: 2025-10-16T22:50:32.349Z Learning: For Convex ESLint configuration in packages/style-kit/src/eslint/convex/config.ts, the file pattern should only include `.js` and `.ts` files (not `.jsx` or `.tsx`), as Convex directories contain backend/server-side code and should not have React component files.Learnt from: drake-nathan Repo: drake-nathan/js-style-kit PR: 88 File: apps/vite-test-app/src/rule-tests/ReactHooks.tsx:45-53 Timestamp: 2025-10-10T00:25:39.030Z Learning: Files in paths like `rule-tests/`, `lint-tests/`, or similar directories that contain React components are typically used to verify ESLint rule configurations and are never mounted or executed. In these test files, intentional violations of React rules (like calling setState during render) are expected and should not be flagged as issues, as they exist specifically to test that the linting rules catch those patterns.packages/eslint-plugin-nextjs/package.json (1)
55-55: LGTM. The devDependency updates (@eslint/core to 1.0.1 and eslint-plugin-eslint-plugin to 7.3.0) are patch and minor versions with low risk to the published package.Also applies to: 60-60
apps/vite-test-app/package.json (1)
17-17: Routine patch updates for test application dependencies.Both
@types/react@19.2.8andvite@7.3.1are valid patch versions. These updates are appropriate for the integration test application.apps/next-test-app/package.json (1)
12-12: LGTM! Routine patch updates for test application.Patch version updates to
convex(1.31.3) and@types/react(19.2.8) are appropriate for this Next.js test application.package.json (1)
30-30: Turbo upgrade to 2.7.3 is valid and safe.turbo@2.7.3 is a valid patch release with no breaking changes. The changelog includes bug fixes (GitHub/git packages support for Bun), internal refactoring, and example improvements.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @scripts/ci.js:
- Line 17: The CI entry currently runs the destructive formatter command; update
the CI job that defines the object with name "format" (the array element {
color: "\x1b[36m", command: "bun run format", name: "format" }) to use the
non-destructive check variant by changing the command to "bun run format:check"
so CI verifies formatting with Prettier --check instead of modifying files.
🧹 Nitpick comments (1)
scripts/ci.js (1)
111-153: Consider adding timeout handling for commands.The
runCommandfunction spawns processes without timeout mechanisms. If a command hangs indefinitely (due to deadlock, waiting for input, or network issues), the CI pipeline will block indefinitely rather than failing fast.⏱️ Example timeout implementation
const runCommand = async (cmd) => { const { command, name } = cmd; // Update status to running status[name].status = "running"; status[name].startTime = Date.now(); commandOutputs[name] = []; printStatus(); return new Promise((resolve) => { + const TIMEOUT_MS = 600000; // 10 minutes + let timeoutId; + const proc = spawn(command, { shell: true, stdio: ["pipe", "pipe", "pipe"], }); + + timeoutId = setTimeout(() => { + proc.kill('SIGTERM'); + commandOutputs[name].push({ + output: `Command timed out after ${TIMEOUT_MS / 1000}s`, + type: "stderr", + }); + }, TIMEOUT_MS); // Collect stdout proc.stdout.on("data", (data) => { const output = data.toString().trim(); commandOutputs[name].push({ output, type: "stdout" }); // Check for Turbo cache hits if (output.includes("cache hit") || output.includes("FULL TURBO")) { status[name].turboCache = true; } }); // Collect stderr proc.stderr.on("data", (data) => { commandOutputs[name].push({ output: data.toString().trim(), type: "stderr", }); }); proc.on("close", (code) => { + clearTimeout(timeoutId); status[name].endTime = Date.now(); status[name].status = code === 0 ? "success" : "failed"; printStatus(); resolve(code === 0); }); }); };
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/style-kit/bin/cli.test.tsscripts/ci.js
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/style-kit/bin/cli.test.ts
🧰 Additional context used
📓 Path-based instructions (1)
scripts/ci.js
📄 CodeRabbit inference engine (CLAUDE.md)
Use scripts/ci.js as the comprehensive CI entry script (format, lint, test, dependency checks, build)
Files:
scripts/ci.js
🧠 Learnings (3)
📓 Common learnings
Learnt from: drake-nathan
Repo: drake-nathan/js-style-kit PR: 84
File: scripts/ci.js:5-10
Timestamp: 2025-09-27T20:21:53.141Z
Learning: In scripts/ci.js for drake-nathan/js-style-kit: The build command must run before lint/format because the build process generates or modifies files that need to be linted/formatted. This script runs locally only and has specific build->lint dependencies.
Learnt from: drake-nathan
Repo: drake-nathan/js-style-kit PR: 84
File: package.json:19-20
Timestamp: 2025-09-27T20:21:27.195Z
Learning: In the js-style-kit monorepo, the root lint script uses --ignore-pattern to exclude packages, apps, and docs because Turborepo runs individual lint scripts for each workspace. The root lint script is only meant to lint root-level files like scripts and config files, while `turbo run lint` handles workspace linting through the dependsOn: ["^lint"] configuration.
Learnt from: CR
Repo: drake-nathan/js-style-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T02:53:12.206Z
Learning: Applies to scripts/ci.js : Use scripts/ci.js as the comprehensive CI entry script (format, lint, test, dependency checks, build)
Learnt from: CR
Repo: drake-nathan/js-style-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T02:53:12.206Z
Learning: Use Turborepo to manage the monorepo build pipeline and caching
Learnt from: CR
Repo: drake-nathan/js-style-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T02:53:12.206Z
Learning: Applies to packages/style-kit/bin/index.ts : Provide the CLI entry point at packages/style-kit/bin/index.ts
📚 Learning: 2025-09-27T20:21:53.141Z
Learnt from: drake-nathan
Repo: drake-nathan/js-style-kit PR: 84
File: scripts/ci.js:5-10
Timestamp: 2025-09-27T20:21:53.141Z
Learning: In scripts/ci.js for drake-nathan/js-style-kit: The build command must run before lint/format because the build process generates or modifies files that need to be linted/formatted. This script runs locally only and has specific build->lint dependencies.
Applied to files:
scripts/ci.js
📚 Learning: 2025-09-12T02:53:12.206Z
Learnt from: CR
Repo: drake-nathan/js-style-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T02:53:12.206Z
Learning: Applies to scripts/ci.js : Use scripts/ci.js as the comprehensive CI entry script (format, lint, test, dependency checks, build)
Applied to files:
scripts/ci.js
🔇 Additional comments (2)
scripts/ci.js (2)
184-191: Early exit on phase failure is appropriate for this workflow.The sequential phase execution with early exit (break on first failure) is a good pattern here because Phase 2 depends on Phase 1 completing successfully. This provides fast feedback and avoids running dependent tasks when prerequisites fail.
The trade-off is that developers won't get comprehensive feedback about all issues in a single CI run if Phase 1 fails, but this is acceptable given the build dependencies between phases.
9-11: Phase 1 parallel execution is correct.Sherif only reads
package.jsonfiles (inputs:["**/package.json"]) and produces no outputs, so it safely runs in parallel withbuildwithout race conditions. The phase structure is sound: Phase 1 runs independent checks (sherif + build), then Phase 2 validates after build completes.
1 participant
Summary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.