chore: Make the Compile Check required status check report on every PR - #1328
Conversation
The branch ruleset will require "Compile Check (Linux)" before merging, but the workflow only triggers on C# path changes and only for PRs targeting main. Without changes, PRs without C# edits (or PRs targeting v3-beta) would never report the check and stay unmergeable. - Add v3-beta to the pull_request branches of the real compile check - Add a no-op twin workflow with the inverse paths-ignore filter so the check name is always reported, following GitHub's documented pattern for skipped-but-required checks
|
Warning Review limit reached
More reviews will be available in 15 minutes and 19 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe Unity compile check workflow is expanded to run on v3-beta pull requests, removes trigger-level path filtering, adds a runtime C# change detection step using git diff, and gates downstream compilation and error check steps on that detection result. A test file adds a using directive. ChangesCompile Check CI Updates for v3-beta
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/unity-compile-check-skip.yml:
- Around line 12-29: The two workflows both declare the job compile-check and
can both run for the same PR; fix by making them mutually exclusive: either
consolidate .github/workflows/unity-compile-check-skip.yml into the main
.github/workflows/unity-compile-check-and-test-runner.yml (remove the duplicate
compile-check job), or keep the skip workflow but make its compile-check job
conditional by adding a preliminary step/job that detects C# changes (via git
diff of the PR or the checkout action) and only proceeds when no C# files are
present (expose that as an output and use job-level if to skip the no-op),
referencing the compile-check job name and the two workflow filenames to locate
the code to change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4704fb26-a33e-47e3-85b2-e341164f5812
📒 Files selected for processing (2)
.github/workflows/unity-compile-check-and-test-runner.yml.github/workflows/unity-compile-check-skip.yml
Running the Unity compile check on v3-beta PRs exposed that the DynamicCodeExecutionScheduler unit tests no longer compiled on this branch. The production scheduler was moved to Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution and its namespace changed to io.github.hatayama.UnityCliLoop.FirstPartyTools, but the test project was never updated, so the workflow's "Run concurrency unit tests" step failed with CS2001 (missing source files) and CS0246 (missing types). This was hidden until now because the workflow only ran on PRs targeting main. - Point the csproj <Compile Include> paths at the relocated source files - Import the FirstPartyTools namespace in the test so the scheduler types resolve, since UnitTests is no longer nested under the source namespace
There was a problem hiding this comment.
1 issue found and verified against the latest diff
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
CodeRabbit and cubic both flagged that the no-op twin workflow and the real compile workflow were not mutually exclusive: GitHub's paths-ignore fires whenever any changed file is outside the ignored set, so a PR that mixes C# and non-C# files triggered both workflows and produced two "Compile Check (Linux)" results. As a required status check, the no-op success could mask the real failure, defeating the merge gate. Consolidate into one always-running workflow. A change-detection step diffs the PR head against its base and only runs the .NET tests and Unity compile when C# sources changed; otherwise the job reports success immediately. This guarantees exactly one Compile Check result per PR. - Remove the workflow-level paths filter and detect C# changes in-job - Gate the .NET and Unity steps on the detection output - Delete the redundant unity-compile-check-skip.yml twin workflow
The architecture test TestPullRequestWorkflowCacheActionsRequireTrustedUnitySecrets requires every actions/cache step in a pull request workflow to carry the exact guard `if: env.HAS_UNITY_LICENSE == 'true'`, so that cache actions never run for untrusted PRs that lack the Unity license secret. The new change-detection gate had combined that guard with the C# detection output, which broke the literal match and failed build-cli. Drop the C# condition from the cache step's `if`. The expensive Unity compile remains gated on C# changes; restoring the Library cache on a non-C# PR is harmless and keeps the security guard intact.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/unity-compile-check-and-test-runner.yml:
- Around line 50-54: The change detection regex (the grep -qE on "$CHANGED" in
unity-compile-check-and-test-runner.yml) only matches .cs, .csproj and the
workflow file, so edits to Unity compile-affecting files (.asmdef,
Packages/manifest.json, Packages/packages-lock.json, etc.) are missed; update
that grep pattern to also include .asmdef and the specific package/manifest
filenames (e.g., Packages/manifest.json, Packages/packages-lock.json) and any
other project metadata files you care about so that when CHANGED contains those
paths the script emits "csharp=true" to GITHUB_OUTPUT.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d785b8af-bca6-4634-9f7e-eb85ff363c1d
📒 Files selected for processing (1)
.github/workflows/unity-compile-check-and-test-runner.yml
CodeRabbit flagged that the change-detection regex only matched .cs and test .csproj files, so a PR touching only compile-shaping metadata could skip the Unity compile yet still report the required check as green. Extend the detection to assembly definitions and package metadata, which the workflow already treats as compile-affecting via the Library cache key: - Packages/**/*.asmdef and Assets/**/*.asmdef - Packages/manifest.json - Packages/packages-lock.json
Summary
Compile Check (Linux)as a merge gate without blocking PRs that do not touch C# code.User Impact
main, so making it a required status check would leave non-C# PRs (and PRs targetingv3-beta) stuck forever waiting for a check that never reports.mainorv3-betareports theCompile Check (Linux)status — either the real compile run (C# changes) or an instant no-op success (no C# changes) — so the required check is always satisfiable.Changes
v3-betato thepull_requestbranch filter of the real Unity compile check workflow.unity-compile-check-skip.yml) that reports the sameCompile Check (Linux)check name via the inversepaths-ignorefilter, following GitHub's documented pattern for skipped-but-required checks.Verification
Compile Check (Linux),build-typescript-server, andtest-unity-packageall run under the github-actions app (integration_id 15368) used by the ruleset.