Repository navigation
chore: Domain editor-settings service wrapper removed in favor of direct port use - #1531
Merged
hatayama merged 2 commits intoJul 6, 2026
Merged
Conversation
All 11 UnityCliLoopEditorSettingsService methods forwarded 1:1 to IUnityCliLoopEditorSettingsPort, and the only precondition in the chain already lives in the sole port implementation (UnityCliLoopEditorSettingsRepository), so the wrapper added no behavior. Consumers across CompositionRoot, Infrastructure, and Presentation now depend on the port directly, with port-based naming throughout. The service file becomes the standalone port interface file (git mv keeps the meta GUID), and the residency/facade guard tests are repointed.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR replaces the editor settings service with a domain port interface and rewires editor composition, startup, presentation, and tests to use the port-backed settings boundary. ChangesSettings Port Migration
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
Packages/src/Editor/Presentation/UnityCliLoopSettingsWindow.cs (1)
1091-1099: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStale "settings service" wording in exception message.
Same issue as
SetupWizardWindow.GetEditorSettingsPort(): the message still reads"Unity CLI Loop editor settings service is not registered."after the type was renamed toIUnityCliLoopEditorSettingsPort.✏️ Proposed fix
- throw new InvalidOperationException("Unity CLI Loop editor settings service is not registered."); + throw new InvalidOperationException("Unity CLI Loop editor settings port is not registered.");🤖 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/Presentation/UnityCliLoopSettingsWindow.cs` around lines 1091 - 1099, The exception message in GetEditorSettingsPort still uses the old “settings service” wording after the rename to IUnityCliLoopEditorSettingsPort. Update the InvalidOperationException text in GetEditorSettingsPort to match the current type/name used in this class, keeping it consistent with SetupWizardWindow.GetEditorSettingsPort and the RegisteredEditorSettingsPort check.Packages/src/Editor/Presentation/Setup/SetupWizardWindow.cs (1)
364-372: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStale "settings service" wording in exception message.
The exception message still says
"Unity CLI Loop editor settings service is not registered."even though the field/type is nowIUnityCliLoopEditorSettingsPort. Purely cosmetic, but could confuse future debugging.✏️ Proposed fix
- throw new System.InvalidOperationException("Unity CLI Loop editor settings service is not registered."); + throw new System.InvalidOperationException("Unity CLI Loop editor settings port is not registered.");🤖 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/Presentation/Setup/SetupWizardWindow.cs` around lines 364 - 372, Update GetEditorSettingsPort so the InvalidOperationException message matches the current IUnityCliLoopEditorSettingsPort naming instead of the stale “settings service” wording. Keep the existing null check on RegisteredEditorSettingsPort, but change the thrown message to reference the editor settings port consistently so future debugging aligns with the type and field names.
🤖 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 `@Packages/src/Editor/Presentation/Setup/SetupWizardWindow.cs`:
- Around line 364-372: Update GetEditorSettingsPort so the
InvalidOperationException message matches the current
IUnityCliLoopEditorSettingsPort naming instead of the stale “settings service”
wording. Keep the existing null check on RegisteredEditorSettingsPort, but
change the thrown message to reference the editor settings port consistently so
future debugging aligns with the type and field names.
In `@Packages/src/Editor/Presentation/UnityCliLoopSettingsWindow.cs`:
- Around line 1091-1099: The exception message in GetEditorSettingsPort still
uses the old “settings service” wording after the rename to
IUnityCliLoopEditorSettingsPort. Update the InvalidOperationException text in
GetEditorSettingsPort to match the current type/name used in this class, keeping
it consistent with SetupWizardWindow.GetEditorSettingsPort and the
RegisteredEditorSettingsPort check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 06be6d50-7c0b-4e58-b2d7-217d5d607149
⛔ Files ignored due to path filters (1)
Packages/src/Editor/Domain/IUnityCliLoopEditorSettingsPort.cs.metais excluded by none and included by none
📒 Files selected for processing (17)
Assets/Tests/Editor/OnionAssemblyDependencyTests.csAssets/Tests/Editor/SetupWizardWindowTests.csAssets/Tests/Editor/StaticFacadeStateGuardTests.csAssets/Tests/Editor/UnityCliLoopEditorSettingsRecoveryTests.csAssets/Tests/Editor/UnityCliLoopEditorSettingsTestFactory.csAssets/Tests/Editor/UnityCliLoopPackageRemovalSettingsResetterTests.csPackages/src/Editor/CompositionRoot/UnityCliLoopApplicationRegistration.csPackages/src/Editor/CompositionRoot/UnityCliLoopEditorBootstrapper.csPackages/src/Editor/Domain/IUnityCliLoopEditorSettingsPort.csPackages/src/Editor/Domain/UnityCliLoopEditorSettingsService.csPackages/src/Editor/Infrastructure/InfrastructureEditorStartup.csPackages/src/Editor/Infrastructure/Settings/UnityCliLoopEditorSettingsRecoveryScheduler.csPackages/src/Editor/Infrastructure/Settings/UnityCliLoopPackageRemovalSettingsResetter.csPackages/src/Editor/Presentation/PresentationEditorStartup.csPackages/src/Editor/Presentation/Setup/SetupWizardWindow.csPackages/src/Editor/Presentation/UnityCliLoopSettingsModel.csPackages/src/Editor/Presentation/UnityCliLoopSettingsWindow.cs
💤 Files with no reviewable changes (2)
- Packages/src/Editor/Domain/UnityCliLoopEditorSettingsService.cs
- Assets/Tests/Editor/StaticFacadeStateGuardTests.cs
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Both GetEditorSettingsPort() accessors threw with wording that referenced the deleted UnityCliLoopEditorSettingsService concept, which would mislead anyone debugging an unregistered-port failure.
hatayama
deleted the
refactor/c4-remove-editor-settings-service-passthrough
branch
July 6, 2026 00:40
RyanXie123
pushed a commit
to RyanXie123/unity-cli-loop
that referenced
this pull request
Sep 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
UnityCliLoopEditorSettingsServicepass-through wrapper from the Domain layer; consumers now depend onIUnityCliLoopEditorSettingsPortdirectly.User Impact
Changes
IUnityCliLoopEditorSettingsPort; the only precondition in the chain already lives in the sole implementationUnityCliLoopEditorSettingsRepository, so nothing is lost.git mv, GUID preserved) into the standaloneIUnityCliLoopEditorSettingsPort.csDomain file, with a boundary-focused doc comment.editorSettingsPortnaming (includingUnityCliLoopApplicationServices.EditorSettingsPortand the window helpers).OnionAssemblyDependencyTestsresidency guard repointed to the port;StaticFacadeStateGuardTestspath list entry for the deleted file removed.Verification
dist/darwin-arm64/uloop compile— 0 errors, 0 warningsuloop run-tests(EditMode,UnityCliLoopEditorSettingsRecoveryTests|UnityCliLoopPackageRemovalSettingsResetterTests|SetupWizardWindowTests|OnionAssemblyDependencyTests|StaticFacadeStateGuardTests) — 221/221 passedgrep -rnw "UnityCliLoopEditorSettingsService"across Packages/, Assets/, tools/ — zero remaining references