-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
bump turbo #19179
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bump turbo #19179
Conversation
WalkthroughThe git utility Pre-merge checks❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/@tailwindcss-standalone/scripts/build.ts (2)
10-11: Consider removing debug logs or gating them behind an environment variable.These console.log statements appear to be temporary debug logs added to diagnose the USERPROFILE issue mentioned in the PR description. Before merging, consider either:
- Removing them if they're no longer needed for debugging
- Gating them behind a debug environment variable (e.g.,
if (process.env.DEBUG_BUILD)) if they provide ongoing value for troubleshooting build issuesApply this diff to gate the logs behind a debug flag:
+ if (process.env.DEBUG_BUILD) { console.log({ USERPROFILE: process.env.USERPROFILE }) console.log({ cwd: process.cwd() }) + }
22-24: Document the USERPROFILE workaround more thoroughly.This workaround addresses the Windows CI build failures mentioned in the PR description. To improve maintainability and facilitate future removal of this workaround:
- Expand the comment to reference the specific issue (e.g., link to a GitHub issue if one was filed with Bun)
- Note any known side effects or limitations of setting USERPROFILE to an empty string
- Consider adding a TODO comment to track when this workaround can be removed
Apply this diff to improve the documentation:
- // This is a fix for binary downloads failing on Windows CI + // Workaround for Bun binary downloads failing on Windows CI when USERPROFILE + // is passed through by Turborepo. See: [issue-link] + // TODO: Remove this workaround once Bun fixes the underlying issue USERPROFILE: '',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
integrations/upgrade/index.test.ts(2 hunks)package.json(1 hunks)packages/@tailwindcss-standalone/scripts/build.ts(2 hunks)packages/@tailwindcss-upgrade/src/utils/git.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/@tailwindcss-upgrade/src/utils/git.ts (1)
playgrounds/v3/scripts/upgrade.mjs (1)
cwd(7-7)
integrations/upgrade/index.test.ts (1)
packages/@tailwindcss-upgrade/src/utils/git.ts (1)
isRepoDirty(3-17)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: macOS / cli
- GitHub Check: macOS / postcss
- GitHub Check: Windows / postcss
- GitHub Check: Windows / vite
- GitHub Check: Windows / cli
- GitHub Check: Windows / upgrade
- GitHub Check: Windows
🔇 Additional comments (3)
packages/@tailwindcss-upgrade/src/utils/git.ts (1)
3-5: LGTM! Clean enhancement for directory-aware repo checks.The optional
cwdparameter is a clean addition that maintains backward compatibility while enabling per-directory git status checks. This aligns well with the test changes inintegrations/upgrade/index.test.tswhere it's used with therootparameter.integrations/upgrade/index.test.ts (1)
2821-2821: LGTM! Test correctly uses the root-aware dirty check.The test callback now properly receives the
rootparameter and passes it toisRepoDirty(root), enabling accurate repository dirty checks scoped to the test's working directory. This aligns well with the API enhancement inpackages/@tailwindcss-upgrade/src/utils/git.ts.Also applies to: 2858-2858
package.json (1)
59-59: Verify whether turbo 2.5.8 was chosen intentionally or if 2.6.1 should be tested.Version 2.5.8 is outdated; the latest available release is 2.6.1. While no security advisories were found, the newer version may address the USERPROFILE-related issues mentioned in the PR. Confirm whether pinning to 2.5.8 was deliberate (e.g., to avoid regressions in 2.6.1) or if 2.6.1 should be evaluated for compatibility.
The Bun build issue was caused by Turborepo passing through
USERPROFILEfrom the env. Probably need to file a bug with Bun: