feat!: Move run logic in separate subcommand#9
Conversation
📝 WalkthroughWalkthroughThe PR extracts the run flow from the inline root command into a dedicated ChangesCLI Subcommand Refactoring
Sequence Diagram(s)sequenceDiagram
participant User
participant RunCMD
participant Logger
participant Config
participant Validator
participant Output
User->>RunCMD: invoke `commitlint-scope run` with flags (--from/--to/--json/--no-color/--verbose)
RunCMD->>Logger: configureLogger (sets debug & AddSource if --verbose)
RunCMD->>Config: validator.LoadConfig()
RunCMD->>Validator: validator.NewValidator(cfg, nil, nil)
RunCMD->>Validator: Validate(commit range)
alt violations found
Validator-->>RunCMD: []violations
alt --json flag set
RunCMD->>Output: jsonOutput(violations)
Output-->>User: indented JSON
else
RunCMD->>Output: textOutput(violations)
Output-->>User: colored grouped text
end
else no violations
RunCMD-->>User: exit success
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
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 docstrings
🧪 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: 1
🤖 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/commands/run.go`:
- Around line 99-116: When violations are found (the violations slice is
non-empty in the run command), return a non-nil error instead of nil so the
process exits with status 1; add a sentinel error (e.g., ErrViolationsFound =
errors.New("violations found")) at package scope and, after printing the red
header and calling jsonOutput or textOutput (referencing flagRunJSON,
jsonOutput, textOutput, cmd.ErrWriter and cmd.Writer), return that sentinel
error (or wrap it) so callers (main.go) can trigger os.Exit(1).
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4dfabf69-95f5-452c-9d68-d67733b2aceb
📒 Files selected for processing (2)
cmd/commands/run.gocmd/root.go
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/errs/violations_found_error.go`:
- Around line 11-13: Update NewViolationsFound to validate that num > 0 and
return an error when it's not: change the signature to NewViolationsFound(num
int) (*ViolationsFoundError, error), check if num <= 0 and return (nil,
fmt.Errorf("invalid num: %d, must be > 0", num)), otherwise return the
constructed &ViolationsFoundError{num: num}, and update any callers (e.g., sites
that pass len(violations)) to handle the (value, error) return accordingly.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7b8e9a02-6626-4443-a572-2a1759bb9908
📒 Files selected for processing (4)
cmd/commands/run.gocmd/errs/violations_found_error.gocmd/root.gomain.go
✅ Files skipped from review due to trivial changes (1)
- main.go
🚧 Files skipped from review as they are similar to previous changes (2)
- cmd/root.go
- cmd/commands/run.go
| func NewViolationsFound(num int) *ViolationsFoundError { | ||
| return &ViolationsFoundError{num: num} | ||
| } |
There was a problem hiding this comment.
Validate that num is positive.
The constructor should validate that num > 0 because a "violations found" error is semantically meaningless with zero or negative violations. While the current call site passes len(violations) (always non-negative), the exported constructor could be misused by future callers.
🛡️ Proposed fix to add validation
func NewViolationsFound(num int) *ViolationsFoundError {
+ if num <= 0 {
+ panic(fmt.Sprintf("ViolationsFoundError requires num > 0, got %d", num))
+ }
return &ViolationsFoundError{num: num}
}Alternatively, return an error from the constructor:
-func NewViolationsFound(num int) *ViolationsFoundError {
+func NewViolationsFound(num int) (*ViolationsFoundError, error) {
+ if num <= 0 {
+ return nil, fmt.Errorf("violations count must be positive, got %d", num)
+ }
return &ViolationsFoundError{num: num}, nil
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func NewViolationsFound(num int) *ViolationsFoundError { | |
| return &ViolationsFoundError{num: num} | |
| } | |
| func NewViolationsFound(num int) *ViolationsFoundError { | |
| if num <= 0 { | |
| panic(fmt.Sprintf("ViolationsFoundError requires num > 0, got %d", num)) | |
| } | |
| return &ViolationsFoundError{num: num} | |
| } |
🤖 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/errs/violations_found_error.go` around lines 11 - 13, Update
NewViolationsFound to validate that num > 0 and return an error when it's not:
change the signature to NewViolationsFound(num int) (*ViolationsFoundError,
error), check if num <= 0 and return (nil, fmt.Errorf("invalid num: %d, must be
> 0", num)), otherwise return the constructed &ViolationsFoundError{num: num},
and update any callers (e.g., sites that pass len(violations)) to handle the
(value, error) return accordingly.
|
🎉 This PR is included in version 2.0.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 2.0.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
1 participant
Summary by CodeRabbit
New Features
runsubcommand to validate commit messages over a specified range.Refactor
Bug Fixes / Behavior