🔀 :: [#272] - 커맨드 계층 리펙토링#273
Conversation
|
Warning Review limit reached
More reviews will be available in 55 minutes and 6 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ 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. WalkthroughCLI 명령 구조를 플랫 루트 기반에서 계층화된 부모-자식 구조로 재편성합니다. 각 카테고리( ChangesCLI 명령 계층 구조 리팩토링
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/unmount.go (1)
16-25:⚠️ Potential issue | 🟠 Major | ⚡ Quick win추가 위치 인자가 조용히 무시됩니다
컨텍스트 전환 시
len(args)검증이 제거되어,unmount호출에 불필요한 추가 인자가 들어와도 실패하지 않고 실행됩니다. 사용자 오입력을 조기에 차단하도록 정확한 인자 개수를 검증해 주세요.수정 예시
RunE: func(cmd *cobra.Command, args []string) error { workspaceId, err := util.GetWorkspaceId(cmd) if err != nil { return cmdError.NewCmdError(1, err.Error()) } + if len(args) != 1 { + return cmdError.NewCmdError(1, "볼륨 아이디가 입력되어야합니다.") + } volumeId, ok := cmd.Context().Value(volumeId).(string) if !ok { return cmdError.NewCmdError(1, "볼륨 아이디가 입력되어야합니다.") }🤖 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 `@cmd/unmount.go` around lines 16 - 25, The RunE handler currently ignores extra positional arguments because len(args) is not validated; add an explicit args count check at the start of the RunE function (the anonymous RunE func in cmd/unmount.go) and return a cmdError.NewCmdError when the count is not the expected value (e.g., expect 0 positional args since volumeId comes from context). Place the check before calling util.GetWorkspaceId and before reading cmd.Context().Value(volumeId) so accidental extra args are rejected early and include a clear error message telling the user that no extra arguments are allowed.
🧹 Nitpick comments (1)
cmd/unmount.go (1)
44-49: ⚡ Quick win
workspace플래그 정의를 부모로 일원화하는 편이 안전합니다
volumeCmd에 이미 persistent--workspace/-w가 있는데unmountCmd에서 동일 플래그를 다시 선언하고 있습니다. 동일 플래그를 두 곳에서 관리하면 설명/기본값이 분기될 수 있어, 자식의 중복 선언은 제거하는 것을 권장합니다.정리 예시
func init() { volumeCmd.AddCommand(unmountCmd) - unmountCmd.Flags().StringP("workspace", "w", "", "워크스페이스 아이디") unmountCmd.Flags().StringP("application", "a", "", "볼륨을 마운트할 애플리케이션 아이디") }🤖 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 `@cmd/unmount.go` around lines 44 - 49, The unmount command currently re-declares the workspace flag already defined as a persistent flag on volumeCmd; remove the duplicate workspace flag registration from init() (the call to unmountCmd.Flags().StringP("workspace", "w", ...)) so unmountCmd inherits the persistent --workspace/-w from volumeCmd, leaving only command-specific flags like the "application" flag in unmountCmd's init; verify any usages of the "workspace" flag in unmountCmd code read the persistent flag via the existing cobra binding rather than a local flag.
🤖 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 `@cmd/application.go`:
- Around line 29-30: The persistent "label" flag on applicationCmd conflicts
with child commands (logs and exec) because those commands still require an
application ID; either move the "label" flag out of applicationCmd and add it as
a local flag only to the deploy, run, and stop commands, or extend the logs and
exec command handlers to accept and resolve the same label identification path:
update the flag definitions and parsing code
(applicationCmd.PersistentFlags().StringArrayP("label", ...), and in cmd/logs.go
and cmd/exec.go modify the command flag handling and resolution logic to check
for the "label" flag first and ignore the explicit application ID when labels
are provided so help text and runtime behavior match.
In `@cmd/deploy.go`:
- Around line 46-47: 루트에서 호출되던 명령 호환이 깨지지 않도록 applicationCmd에만 등록된 deployCmd를
루트에도 등록하거나 루트용 deprecated alias를 추가해 기존 "dcd deploy ..." 호출을 유지하세요; 대상 심볼은
applicationCmd, deployCmd(및 동일한 패턴의 run/stop/exec/logs 커맨드)이며, 루트에 등록할 때에는 사용자에게
마이그레이션 안내(명확한 deprecation/migration 에러 메시지)를 같이 출력하도록 구현하세요.
---
Outside diff comments:
In `@cmd/unmount.go`:
- Around line 16-25: The RunE handler currently ignores extra positional
arguments because len(args) is not validated; add an explicit args count check
at the start of the RunE function (the anonymous RunE func in cmd/unmount.go)
and return a cmdError.NewCmdError when the count is not the expected value
(e.g., expect 0 positional args since volumeId comes from context). Place the
check before calling util.GetWorkspaceId and before reading
cmd.Context().Value(volumeId) so accidental extra args are rejected early and
include a clear error message telling the user that no extra arguments are
allowed.
---
Nitpick comments:
In `@cmd/unmount.go`:
- Around line 44-49: The unmount command currently re-declares the workspace
flag already defined as a persistent flag on volumeCmd; remove the duplicate
workspace flag registration from init() (the call to
unmountCmd.Flags().StringP("workspace", "w", ...)) so unmountCmd inherits the
persistent --workspace/-w from volumeCmd, leaving only command-specific flags
like the "application" flag in unmountCmd's init; verify any usages of the
"workspace" flag in unmountCmd code read the persistent flag via the existing
cobra binding rather than a local flag.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2880291e-dcdf-4a04-b82c-af5ce5867a88
📒 Files selected for processing (13)
cmd/application.gocmd/connect.gocmd/deploy.gocmd/disconnect.gocmd/domain.gocmd/exec.gocmd/logs.gocmd/mount.gocmd/root.gocmd/run.gocmd/stop.gocmd/unmount.gocmd/volume.go
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
cmd/stop.go (1)
13-13:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse 필드가 실제 사용법과 일치하지 않습니다.
Use필드에"stop <applicationId> [flags]"로 표시되어 있지만, 이제applicationId는 부모 커맨드(application)에 전달됩니다. 실제 사용법은application <applicationId> stop [flags]이므로, 자식 커맨드의Use필드는"stop [flags]"로 수정되어야 합니다.📝 Use 필드 수정 제안
- Use: "stop <applicationId> [flags]", + Use: "stop [flags]",🤖 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 `@cmd/stop.go` at line 13, The command's Use string still shows "stop <applicationId> [flags]" but applicationId is provided by the parent application command; update the child Cobra command's Use field (the stop command declaration, e.g., the variable defining the stop cobra.Command) to "stop [flags]" so the usage matches the actual invocation "application <applicationId> stop [flags]". Ensure only the Use value is changed and leave other fields unchanged.cmd/run.go (1)
13-13:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse 필드가 실제 사용법과 일치하지 않습니다.
Use필드에"run <applicationId> [flags]"로 표시되어 있지만, 이제applicationId는 부모 커맨드(application)에 전달됩니다. 실제 사용법은application <applicationId> run [flags]이므로, 자식 커맨드의Use필드는"run [flags]"로 수정되어야 합니다.📝 Use 필드 수정 제안
- Use: "run <applicationId> [flags]", + Use: "run [flags]",🤖 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 `@cmd/run.go` at line 13, Update the Cobra command `Use` string for the run subcommand in cmd/run.go: replace the existing `Use: "run <applicationId> [flags]"` with `Use: "run [flags]"` so it reflects the new parent-level `application <applicationId> run [flags]` usage; locate the Cobra command definition (the variable that initializes &cobra.Command for the run command) and change its `Use` field accordingly.
♻️ Duplicate comments (1)
cmd/stop.go (1)
49-49:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift플래그 등록 패턴이 다른 자식 커맨드와 일관되지 않습니다.
run.go와 동일한 문제입니다.deploy.go에서는label플래그를applicationCmd.PersistentFlags()에 등록하지만, 여기서는stopCmd.PersistentFlags()에 등록하고 있습니다. 이로 인해:
- 각 자식 커맨드가 동일한 플래그를 중복 정의하게 됩니다
- 잠재적인 플래그 충돌이나 예상치 못한 동작이 발생할 수 있습니다
- 형제 커맨드 간 API 계약이 불일치합니다
모든
application자식 커맨드에서label플래그를 사용할 수 있어야 한다면, 한 번만applicationCmd.PersistentFlags()에 등록하는 것이 올바른 패턴입니다.♻️ 일관된 패턴으로 수정 제안
stop.go의init()함수에서 플래그 등록을 제거하고,deploy.go처럼application.go의init()에서 한 번만 등록하도록 수정:func init() { applicationCmd.AddCommand(stopCmd) - - stopCmd.PersistentFlags().StringArrayP("label", "l", []string{}, "애플리케이션을 식별하기위한 라벨.\n이 플래그를 사용할때 명시한 애플리케이션 아이디는 무시됩니다.\nex). -l test-label-1 -l test-label-2") }🤖 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 `@cmd/stop.go` at line 49, The stop command registers the "label" flag on stopCmd.PersistentFlags(), causing duplicate/conflicting flags across sibling commands; remove the stopCmd.PersistentFlags().StringArrayP(...) registration from stop.go's init() and instead ensure the "label" flag is registered exactly once on applicationCmd.PersistentFlags() (as done in deploy.go/run.go), so update application.go's init() to include the StringArrayP("label", "l", ...) registration if it's not already present and delete the corresponding registration in stop.go.
🤖 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 `@cmd/run.go`:
- Line 50: The "label" persistent flag is being registered multiple times
(applicationCmd.PersistentFlags(), runCmd.PersistentFlags(),
stopCmd.PersistentFlags()), causing inconsistent Cobra merging; keep a single
shared registration on applicationCmd by removing the duplicate registrations
from run.go and stop.go (delete the runCmd.PersistentFlags().StringArrayP(...)
and stopCmd.PersistentFlags().StringArrayP(...) calls) so only
applicationCmd.PersistentFlags().StringArrayP("label", ...) defines the flag for
all application subcommands.
---
Outside diff comments:
In `@cmd/run.go`:
- Line 13: Update the Cobra command `Use` string for the run subcommand in
cmd/run.go: replace the existing `Use: "run <applicationId> [flags]"` with `Use:
"run [flags]"` so it reflects the new parent-level `application <applicationId>
run [flags]` usage; locate the Cobra command definition (the variable that
initializes &cobra.Command for the run command) and change its `Use` field
accordingly.
In `@cmd/stop.go`:
- Line 13: The command's Use string still shows "stop <applicationId> [flags]"
but applicationId is provided by the parent application command; update the
child Cobra command's Use field (the stop command declaration, e.g., the
variable defining the stop cobra.Command) to "stop [flags]" so the usage matches
the actual invocation "application <applicationId> stop [flags]". Ensure only
the Use value is changed and leave other fields unchanged.
---
Duplicate comments:
In `@cmd/stop.go`:
- Line 49: The stop command registers the "label" flag on
stopCmd.PersistentFlags(), causing duplicate/conflicting flags across sibling
commands; remove the stopCmd.PersistentFlags().StringArrayP(...) registration
from stop.go's init() and instead ensure the "label" flag is registered exactly
once on applicationCmd.PersistentFlags() (as done in deploy.go/run.go), so
update application.go's init() to include the StringArrayP("label", "l", ...)
registration if it's not already present and delete the corresponding
registration in stop.go.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 94592f19-e267-4f8e-b373-df6e0581fba0
📒 Files selected for processing (4)
cmd/application.gocmd/deploy.gocmd/run.gocmd/stop.go
💤 Files with no reviewable changes (1)
- cmd/application.go
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/deploy.go
1 participant
개요
작업내용
체크리스트
Summary by CodeRabbit