-
Notifications
You must be signed in to change notification settings - Fork 19
fix #22 : convert LLM provider selection to switch statement #41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Replaced if-else with switch statement to explicitly handle all providers (gemini, chatgpt, claude, grok) - Used existing commitLLM variable instead of multiple os.Getenv calls
…tch-statement fix DFanso#22 : convert LLM provider selection to switch statement
WalkthroughRefactors commit message generation dispatch in Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant CLI as commit-msg CLI
participant Gemini as Gemini Provider
participant ChatGPT as ChatGPT Provider
participant Claude as Claude Provider
participant Grok as Grok Provider
Dev->>CLI: Run commit message generation
CLI->>CLI: Read COMMIT_LLM -> commitLLM
alt commitLLM == "gemini"
CLI->>Gemini: Generate message (Gemini API key)
Gemini-->>CLI: Commit message
else commitLLM == "chatgpt"
CLI->>ChatGPT: Generate message (OpenAI API key)
ChatGPT-->>CLI: Commit message
else commitLLM == "claude"
CLI->>Claude: Generate message (Anthropic API key)
Claude-->>CLI: Commit message
else
CLI->>Grok: Generate message (Grok API key)
Grok-->>CLI: Commit message
end
CLI-->>Dev: Output commit message
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 (1)
cmd/commit-msg/main.go (1)
50-152: Fix inconsistent indentation.The code block from line 50 onwards has extra indentation (appears to use tabs where it shouldn't). This is inconsistent with the indentation of the switch statements above and affects code readability.
Run
gofmtor your IDE's formatter to correct the indentation:#!/bin/bash # Format the Go code gofmt -w cmd/commit-msg/main.go
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cmd/commit-msg/main.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
cmd/commit-msg/main.go (4)
internal/claude/claude.go (1)
GenerateCommitMessage(33-85)internal/chatgpt/chatgpt.go (1)
GenerateCommitMessage(13-32)internal/gemini/gemini.go (1)
GenerateCommitMessage(13-44)internal/grok/grok.go (1)
GenerateCommitMessage(15-87)
🔇 Additional comments (1)
cmd/commit-msg/main.go (1)
25-48: LGTM with a caveat.The switch statement correctly validates the LLM provider and retrieves the appropriate API key. However, note that the default case (line 47) will exit the program for any unrecognized or empty
COMMIT_LLMvalue, which impacts the second switch statement's default case (see next comment).
DFanso
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adeeshperera can u check the suggestion and fix the issue
Removed the explicit Grok case, keeping Grok as the default fallback provider
refactor: remove explicit Grok case from LLM provider switch
DFanso
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🎊
Summary by CodeRabbit