Skip to content

chore: Simplify native CLI installer command construction - #1619

Merged
hatayama merged 1 commit into
v3-betafrom
refactor/hatayama/split-native-cli-installer
Jul 8, 2026
Merged

hatayama merged 1 commit into
v3-betafrom
refactor/hatayama/split-native-cli-installer

Conversation

@hatayama

@hatayama hatayama commented Jul 8, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Preserve native CLI installer behavior while separating command construction from process and path responsibilities.
  • Keep the existing public installer command API unchanged.

User Impact

  • No user-visible behavior change.
  • Setup and uninstall continue to produce the same executable names, arguments, manual commands, release tags, and installer script URLs.

Changes

  • Move install command construction and quoting helpers into NativeCliCommandBuilder.
  • Keep NativeCliInstaller.GetInstallCommand as the public facade.
  • Keep uninstall path resolution in NativeCliInstaller until the path responsibility is split separately.
  • Retarget command-focused tests and the overload guard to the extracted builder; the guard still reports missing targets.

Verification

  • Unity compile: 0 errors, 0 warnings
  • NativeCliInstallerTests: 32 passed
  • StaticFacadeStateGuardTests: 20 passed
  • CliSetupApplicationServiceTests: 4 passed
  • git diff --check
  • No IPC or protocol changes

Move installer command assembly into a focused builder while preserving the public facade and command behavior. Keep uninstall path resolution in NativeCliInstaller until path responsibilities are separated.
@coderabbitai

coderabbitai Bot commented Jul 8, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This PR extracts install-command and installer-script URL construction logic from NativeCliInstaller into a new NativeCliCommandBuilder class. NativeCliInstaller now delegates to the new builder for command construction, quoting, and URL generation, with corresponding tests and an overload-guard allowlist updated to reference the new file.

Changes

Command Builder Extraction

Layer / File(s) Summary
New command builder core
Packages/src/Editor/Infrastructure/CLI/NativeCliCommandBuilder.cs
New internal static class builds remote/local install commands for POSIX and Windows platforms, branching on RuntimePlatform.
Path resolution and quoting/URL helpers
Packages/src/Editor/Infrastructure/CLI/NativeCliCommandBuilder.cs
Adds local installer script path resolution, POSIX/PowerShell/process argument quoting utilities, release tag normalization, and installer script URL construction.
NativeCliInstaller delegation
Packages/src/Editor/Infrastructure/CLI/NativeCliInstaller.cs
GetInstallCommand and BuildUninstallCommand now delegate to NativeCliCommandBuilder; ~174 lines of duplicated private helper methods are removed.
Test and guard updates
Assets/Tests/Editor/NativeCliInstallerTests.cs, Assets/Tests/Editor/StaticFacadeStateGuardTests.cs
Tests switch command/URL construction calls to NativeCliCommandBuilder; the overload guard allowlist path is updated to target the new file.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

  • hatayama/unity-cli-loop#1032: Both modify NativeCliInstaller's install-command construction path, one refactoring it into NativeCliCommandBuilder, the other adding legacy-removal logic to it.
  • hatayama/unity-cli-loop#1169: Both touch BuildUninstallCommand/uninstall behavior in NativeCliInstaller, one moving quoting into NativeCliCommandBuilder.
  • hatayama/unity-cli-loop#1190: Both change installer-script URL construction for stable vs beta release tags in the same code path.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly matches the refactor to simplify native CLI installer command construction.
Description check ✅ Passed The description is directly related to the changeset and accurately summarizes the refactor and verification.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/hatayama/split-native-cli-installer

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 5 files

Re-trigger cubic

@coderabbitai

coderabbitai Bot commented Jul 8, 2026 •

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Repository owner deleted a comment from qua-hatayama Jul 8, 2026
@hatayama
hatayama merged commit 8f4b913 into v3-beta Jul 8, 2026
10 checks passed
@hatayama
hatayama deleted the refactor/hatayama/split-native-cli-installer branch July 8, 2026 13:52
RyanXie123 pushed a commit to RyanXie123/unity-cli-loop that referenced this pull request Sep 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant