Repository navigation
chore: Pending compile session state uses a dedicated repository - #1533
Conversation
Add a pending compile session repository behind a Domain port and share SessionState key/index helpers with the compile-result repository so the transition avoids a third persistence helper copy.
|
Warning Review limit reached
Next review available in: 21 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughPending compile request persistence is extracted from ChangesPending Compile Session Repository Extraction
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Registration as UnityCliLoopApplicationRegistration
participant Service as UnityCliLoopEditorSessionStateService
participant PendingRepo as UnityCliLoopPendingCompileSessionRepository
participant Storage as UnityCliLoopEditorSessionStateStorage
Registration->>PendingRepo: new UnityCliLoopPendingCompileSessionRepository()
Registration->>Service: new UnityCliLoopEditorSessionStateService(sessionStatePort, compileResultRepo, pendingRepo)
Service->>PendingRepo: StorePendingCompileRequest(requestId, forceRecompile, expiresAtUtcTicks, reloadObserved)
PendingRepo->>Storage: AddRequestIdToIndex / SetString / SetBool
Service->>PendingRepo: GetPendingCompileRequestForRequestId(requestId)
PendingRepo->>Storage: ParseUtcTicks / GetString / GetBool
Storage-->>PendingRepo: parsed values
PendingRepo-->>Service: UnityCliLoopPendingCompileRequest
Service->>PendingRepo: ClearExpiredPendingCompileRequest(utcNow)
PendingRepo->>Storage: RemoveRequestIdFromIndex
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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.
🧹 Nitpick comments (2)
Assets/Tests/Editor/UnityCliLoopEditorSessionStateRepositoryTests.cs (1)
218-222: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the test factory instead of manual construction.
Every other test in this file recreates the service via
UnityCliLoopEditorSessionStateTestFactory.CreateService(); this test constructs it manually with all three repositories, duplicating the factory's logic and risking drift if the constructor signature changes again.♻️ Proposed refactor
- UnityCliLoopEditorSessionStateService recreatedService = - new UnityCliLoopEditorSessionStateService( - new UnityCliLoopEditorSessionStateRepository(), - new UnityCliLoopCompileResultSessionRepository(), - new UnityCliLoopPendingCompileSessionRepository()); + UnityCliLoopEditorSessionStateService recreatedService = + UnityCliLoopEditorSessionStateTestFactory.CreateService();🤖 Prompt for 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. In `@Assets/Tests/Editor/UnityCliLoopEditorSessionStateRepositoryTests.cs` around lines 218 - 222, Replace the manual recreation of UnityCliLoopEditorSessionStateService in this test with UnityCliLoopEditorSessionStateTestFactory.CreateService() so it matches the rest of the file and centralizes construction logic. Update the test to use the existing factory instead of directly instantiating UnityCliLoopEditorSessionStateRepository, UnityCliLoopCompileResultSessionRepository, and UnityCliLoopPendingCompileSessionRepository, keeping the setup consistent if the service constructor changes.Packages/src/Editor/Infrastructure/Settings/UnityCliLoopEditorSessionStateRepository.cs (1)
112-120: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused
GetString/SetStringhelpers.UnityCliLoopEditorSessionStateRepositoryno longer uses them, so they’re dead code.🤖 Prompt for 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. In `@Packages/src/Editor/Infrastructure/Settings/UnityCliLoopEditorSessionStateRepository.cs` around lines 112 - 120, Remove the dead helper methods GetString and SetString from UnityCliLoopEditorSessionStateRepository since they are no longer referenced anywhere in the class. Update the repository implementation to rely only on the remaining SessionState usage, and make sure no callers depend on these private helpers before deleting them.
🤖 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.
Nitpick comments:
In `@Assets/Tests/Editor/UnityCliLoopEditorSessionStateRepositoryTests.cs`:
- Around line 218-222: Replace the manual recreation of
UnityCliLoopEditorSessionStateService in this test with
UnityCliLoopEditorSessionStateTestFactory.CreateService() so it matches the rest
of the file and centralizes construction logic. Update the test to use the
existing factory instead of directly instantiating
UnityCliLoopEditorSessionStateRepository,
UnityCliLoopCompileResultSessionRepository, and
UnityCliLoopPendingCompileSessionRepository, keeping the setup consistent if the
service constructor changes.
In
`@Packages/src/Editor/Infrastructure/Settings/UnityCliLoopEditorSessionStateRepository.cs`:
- Around line 112-120: Remove the dead helper methods GetString and SetString
from UnityCliLoopEditorSessionStateRepository since they are no longer
referenced anywhere in the class. Update the repository implementation to rely
only on the remaining SessionState usage, and make sure no callers depend on
these private helpers before deleting them.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 63d90568-0e28-4f88-baf5-5d0bba127959
⛔ Files ignored due to path filters (3)
Packages/src/Editor/Domain/IPendingCompileSessionRepository.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/Settings/UnityCliLoopEditorSessionStateStorage.cs.metais excluded by none and included by nonePackages/src/Editor/Infrastructure/Settings/UnityCliLoopPendingCompileSessionRepository.cs.metais excluded by none and included by none
📒 Files selected for processing (10)
Assets/Tests/Editor/OnionAssemblyDependencyTests.csAssets/Tests/Editor/UnityCliLoopEditorSessionStateRepositoryTests.csAssets/Tests/Editor/UnityCliLoopEditorSessionStateTestFactory.csPackages/src/Editor/CompositionRoot/UnityCliLoopApplicationRegistration.csPackages/src/Editor/Domain/IPendingCompileSessionRepository.csPackages/src/Editor/Domain/UnityCliLoopEditorSessionStateService.csPackages/src/Editor/Infrastructure/Settings/UnityCliLoopCompileResultSessionRepository.csPackages/src/Editor/Infrastructure/Settings/UnityCliLoopEditorSessionStateRepository.csPackages/src/Editor/Infrastructure/Settings/UnityCliLoopEditorSessionStateStorage.csPackages/src/Editor/Infrastructure/Settings/UnityCliLoopPendingCompileSessionRepository.cs
Summary
Changes
Verification