feat: mobilecli doctor for better troubleshooting #138
Conversation
WalkthroughIntroduces a new "doctor" CLI subcommand that performs system diagnostics. Adds a GetVersion() function to retrieve the module version and creates a commands/doctor.go file with DoctorCommand and DoctorInfo struct to collect environment diagnostic information including OS details, Android SDK paths, and Xcode configuration. Changes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
cli/doctor.go (1)
14-21: Simplify error formatting.The
fmt.Errorf("%s", response.Error)pattern is redundant whenresponse.Erroris already a string.🔎 Apply this diff to simplify:
RunE: func(cmd *cobra.Command, args []string) error { response := commands.DoctorCommand(GetVersion()) printJson(response) if response.Status == "error" { - return fmt.Errorf("%s", response.Error) + return fmt.Errorf(response.Error) } return nil },commands/doctor.go (1)
129-170: Clarify the distinction and consider consolidating.Both
getXcodePath()andgetXcodeCLIToolsPath()execute the samexcode-select -pcommand:
getXcodePath()filters for paths containing "Xcode.app" (full Xcode installation)getXcodeCLIToolsPath()returns any valid path (including standalone Command Line Tools)This creates two concerns:
- The distinction between these functions isn't immediately clear from their names
- The
xcode-select -pcommand is executed twice unnecessarilyConsider consolidating the logic or documenting the intended difference more clearly.
🔎 View suggested refactor:
+func getXcodeSelectPath() string { + if runtime.GOOS != "darwin" { + return "" + } + + cmd := exec.Command("xcode-select", "-p") + output, err := cmd.CombinedOutput() + if err != nil { + return "" + } + + path := strings.TrimSpace(string(output)) + if _, err := os.Stat(path); err == nil { + return path + } + return "" +} + func getXcodePath() string { - if runtime.GOOS != "darwin" { - return "" - } - - // check if Xcode.app is installed - cmd := exec.Command("xcode-select", "-p") - output, err := cmd.CombinedOutput() - if err != nil { - return "" - } - - path := strings.TrimSpace(string(output)) - + path := getXcodeSelectPath() // check if this is the full Xcode.app path if strings.Contains(path, "Xcode.app") { return path } return "" } func getXcodeCLIToolsPath() string { - if runtime.GOOS != "darwin" { - return "" - } - - cmd := exec.Command("xcode-select", "-p") - output, err := cmd.CombinedOutput() - if err != nil { - return "" - } - - path := strings.TrimSpace(string(output)) - - // verify the path exists - if _, err := os.Stat(path); err == nil { - return path - } - - return "" + return getXcodeSelectPath() }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cli/doctor.go(1 hunks)cli/root.go(1 hunks)commands/doctor.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
cli/doctor.go (2)
commands/doctor.go (1)
DoctorCommand(226-249)cli/root.go (1)
GetVersion(132-134)
commands/doctor.go (1)
commands/commands.go (2)
CommandResponse(11-15)NewSuccessResponse(18-23)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: lint
🔇 Additional comments (10)
cli/root.go (1)
131-134: LGTM!The
GetVersion()function correctly exposes the internal version constant for use by the doctor command.cli/doctor.go (1)
24-26: LGTM!The command registration follows standard Cobra patterns.
commands/doctor.go (8)
11-22: LGTM!The
DoctorInfostruct is well-structured with appropriate JSON tags, and correctly uses a pointer type for the optional boolean field.
24-62: LGTM!The function correctly checks
ANDROID_HOMEand falls back to platform-specific default locations, with proper path validation.
64-84: LGTM!The function correctly locates
adbfrom the Android SDK or system PATH, with appropriate Windows executable suffix handling.
86-105: LGTM!The function follows the same pattern as
getAdbPath(), correctly locating the emulator binary.
107-127: LGTM!The function appropriately handles the
adb versioncommand output and gracefully returns empty strings on errors, which is suitable for diagnostic purposes.
172-189: LGTM!The function appropriately checks
DevToolsSecuritystatus on macOS and uses a pointer to bool for proper optional handling in JSON serialization.
191-223: LGTM!The function correctly handles OS version retrieval for Darwin, Windows, and Linux using platform-appropriate methods.
225-249: LGTM!The
DoctorCommand()function properly aggregates diagnostic information with appropriate platform-specific conditional logic, and returns a well-structured response.
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.