Skip to content

refactor: move 'crashes' commands under 'device crashes'#209

Open
gmegidish wants to merge 1 commit intomainfrom
refactor/device-crashes
Open

refactor: move 'crashes' commands under 'device crashes'#209
gmegidish wants to merge 1 commit intomainfrom
refactor/device-crashes

Conversation

@gmegidish
Copy link
Copy Markdown
Member

No description provided.

…crashes

- add device crashes list and device crashes get subcommands
- mark root crashes command as deprecated (warns via stderr, hidden from help)
- update root.go examples and README to use new device crashes path
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 30, 2026

Walkthrough

This change restructures crash report CLI commands by moving them under the device namespace. The old crashes command is marked as deprecated in favor of device crashes. A new device_crashes.go module implements list and get subcommands that call corresponding backend commands and handle error responses. Documentation and help text in README.md and root.go are updated to reflect the new command paths. The --device flag requirement is preserved across the refactored commands.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive No pull request description was provided by the author, making it impossible to evaluate whether the description relates to the changeset. Add a brief description explaining the rationale for moving crash commands under the 'device' namespace and any relevant context.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main refactoring: moving crash commands from the root 'crashes' namespace to under 'device crashes'.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/device-crashes

Review rate limit: 4/5 reviews remaining, refill in 12 minutes.

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
cli/device_crashes.go (1)

15-39: ⚡ Quick win

Avoid duplicating crash handler logic in both command paths.

Line 19-24 and Line 33-38 duplicate the same response/error handling already present in cli/crashes.go (Line 20-38). During deprecation, this is easy to drift. Consider extracting shared runCrashesList / runCrashesGet helpers and wiring both crashes and device crashes to them.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli/device_crashes.go` around lines 15 - 39, Extract the duplicated response
handling into shared helpers—create runCrashesList(deviceId string) error and
runCrashesGet(deviceId, id string) error that call commands.CrashesListCommand /
commands.CrashesGetCommand, call printJson(response), check response.Status ==
"error" and return fmt.Errorf(response.Error) otherwise nil; then replace the
inline bodies of deviceCrashesListCmd.RunE and deviceCrashesGetCmd.RunE to
delegate to these helpers (and wire the existing crashes commands to use the
same helpers) so the logic in the CLI is centralized and not duplicated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@cli/device_crashes.go`:
- Around line 15-39: Extract the duplicated response handling into shared
helpers—create runCrashesList(deviceId string) error and runCrashesGet(deviceId,
id string) error that call commands.CrashesListCommand /
commands.CrashesGetCommand, call printJson(response), check response.Status ==
"error" and return fmt.Errorf(response.Error) otherwise nil; then replace the
inline bodies of deviceCrashesListCmd.RunE and deviceCrashesGetCmd.RunE to
delegate to these helpers (and wire the existing crashes commands to use the
same helpers) so the logic in the CLI is centralized and not duplicated.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fe3a6d1d-49bc-4cb9-9300-1db348fe1813

📥 Commits

Reviewing files that changed from the base of the PR and between bd327de and 66d1e2a.

📒 Files selected for processing (4)
  • README.md
  • cli/crashes.go
  • cli/device_crashes.go
  • cli/root.go

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