revert: undo CI split approach (#2092)#2093
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Review Summary
(1) Total Issues | Risk: Low
🟡 Minor (1) 🟡
Inline Comments:
- 🟡 Minor:
ci.yml:108-114Comment slightly overstates parallelism behavior
💡 APPROVE WITH SUGGESTIONS
Summary: This is a clean revert that consolidates the CI workflow back to relying on Turborepo's dependency graph rather than manually splitting build and test phases. The approach is sound — Turborepo's dependsOn declarations in turbo.json already ensure proper ordering (builds complete before tests), and the Remote Cache should prevent redundant rebuilds on cache-hit runs.
The one minor suggestion is to clarify the inline comments to accurately reflect task dependency behavior (lint runs immediately, typecheck and test wait for build). This prevents future confusion when debugging CI timing.
Looking forward to the follow-up PR with Turborepo-native concurrency constraints! 🚀
Discarded (2)
| Location | Issue | Reason Discarded |
|---|---|---|
ci.yml |
CPU contention may resurface on cold cache | Informational/advisory only — PR description already acknowledges follow-up PR is planned with proper fix. |
ci.yml |
Actions pinned to mutable tags | Pre-existing issue not introduced by this PR — out of scope. |
Reviewers (1)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-devops |
4 | 0 | 0 | 0 | 1 | 0 | 2 |
| Total | 4 | 0 | 0 | 0 | 1 | 0 | 2 |
Note: 1 finding was positive validation (revert safety), not included in counts.
| # Run all CI checks in parallel with Turborepo | ||
| - name: Run CI checks | ||
| id: ci-check | ||
| run: pnpm check | ||
| run: | | ||
| # Run all checks in parallel using Turborepo's dependency graph | ||
| # This will build once and cache, then run lint, typecheck, and test in parallel | ||
| pnpm check |
There was a problem hiding this comment.
🟡 Minor: Comment slightly overstates parallelism
Issue: The comment states Turborepo will "run lint, typecheck, and test in parallel" after building. However, per turbo.json, lint (line 92) has no build dependency, typecheck (line 102) depends on ^build, and test (line 55) depends on build. On cache-miss runs, lint runs immediately while typecheck and test wait for their dependencies.
Why: The current comment could mislead someone debugging CI timing issues into thinking all checks run concurrently from the start.
Fix: (1-click apply)
| # Run all CI checks in parallel with Turborepo | |
| - name: Run CI checks | |
| id: ci-check | |
| run: pnpm check | |
| run: | | |
| # Run all checks in parallel using Turborepo's dependency graph | |
| # This will build once and cache, then run lint, typecheck, and test in parallel | |
| pnpm check | |
| # Run all CI checks via Turborepo | |
| - name: Run CI checks | |
| id: ci-check | |
| run: | | |
| # Turborepo handles task dependencies: lint runs immediately, | |
| # while typecheck and test wait for build to complete (cached when unchanged). | |
| pnpm check |
Refs:
1 participant
Summary
Reverts #2092 (splitting build and test phases in CI) based on Andrew's feedback to instead use Turborepo's built-in dependency system + concurrency constraints, which will properly respond to cache hits/misses rather than hard-coding a two-step pipeline.
A replacement PR with the proper Turborepo-native approach will follow.