Skip to content

Git#4

Merged
thumbrise merged 3 commits into
mainfrom
git
May 27, 2026
Merged

Git#4
thumbrise merged 3 commits into
mainfrom
git

Conversation

@thumbrise
Copy link
Copy Markdown
Owner

@thumbrise thumbrise commented May 27, 2026

Summary by CodeRabbit

Release Notes

  • Refactor

    • Restructured validator initialization with improved configuration options and cleaner API.
    • Enhanced git command integration with better error handling and contextual logging support.
  • Tests

    • Added comprehensive test coverage for git operations including SHA resolution, commit messages, and changed file tracking.

Review Change Stack

thumbrise added 3 commits May 27, 2026 21:28
- Replace validator args to options struct
- Move validator interfaces to validator.go
- Add default implementations if options fields not set

Need to implement OutsiderFinder, ScopeParser
@thumbrise thumbrise self-assigned this May 27, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 27, 2026

📝 Walkthrough

Walkthrough

The PR refactors the validator package's construction API by moving from positional arguments to an Options-based pattern, consolidates public types into validator.go, adds a concrete DefaultGit implementation wrapping git CLI commands, and updates the command and tests to use the new API.

Changes

Validator API and Git Implementation

Layer / File(s) Summary
Validator API types and Options struct
pkg/validator/validator.go
Violation struct and Git, ScopeParser, OutsiderFinder interfaces are defined in validator.go alongside a new Options struct bundling logger, SHA length, and dependency implementations.
NewValidator constructor with defaults
pkg/validator/validator.go
NewValidator now accepts Options instead of positional arguments; defaults logger to slog.Default(), git to DefaultGit("") when nil, defaults SHALength to 7 when zero, and panics on nil OutsiderFinder or ScopeParser.
DefaultGit CLI integration and tests
pkg/validator/git.go, pkg/validator/git_test.go
DefaultGit wraps git CLI commands for SHA range resolution (git rev-list), commit message retrieval (git log --format=%s), and changed file listing (git diff-tree). Tests cover all three methods, context cancellation, and multi-commit scenarios in isolated temporary repositories.
Test updates for Options-based constructor
pkg/validator/validator_test.go
TestValidator_OneViolation and TestValidator_Scenarios construct and pass validator.Options struct to NewValidator instead of using positional arguments.
Command imports, validator initialization, and output routing
cmd/root.go
Imports log/slog and validator package; initializes validator via NewValidator(Options{...}); redirects JSON output to cmd.ErrWriter instead of os.Stdout.

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly Related PRs

  • thumbrise/commitlint-scope#1: Modifies cmd/root.go to implement CLI wiring with Violation and ScopeValidator types that this PR refactors.
  • thumbrise/commitlint-scope#2: Introduces the original Validator implementation with positional-argument constructor and types that this PR refactors into Options pattern.

A refactor hops in with grace,
Options neat in one small place,
Git commands run with flair,
Tests confirm it all works fair,
Validation flows so clean and bright, 🐰

🚥 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.
Title check ❓ Inconclusive The title 'Git' is vague and does not clearly convey the main purpose of the changes, which involve introducing a concrete Git implementation and refactoring the validator API. Use a more descriptive title that summarizes the primary change, such as 'Introduce DefaultGit implementation and refactor validator API' or 'Replace Git interface with concrete DefaultGit implementation'.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch git

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


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.

❤️ Share

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.

Actionable comments posted: 1

🧹 Nitpick comments (2)
pkg/validator/git.go (1)

52-60: 💤 Low value

Fix grammatical error in comment.

The comment on line 53 contains a grammatical error: "no another way" should be "no other way".

✏️ Proposed fix
-	//nolint:gosec // no another way
+	//nolint:gosec // no other way
🤖 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 `@pkg/validator/git.go` around lines 52 - 60, Fix the grammatical error in the
inline nolint comment in the gitCommand function: change the comment
"//nolint:gosec // no another way" to use correct wording ("no other way") so it
reads e.g. "//nolint:gosec // no other way" in the DefaultGit.gitCommand
function to keep the nolint rationale grammatically correct.
cmd/root.go (1)

68-68: 💤 Low value

Confirm intentional use of ErrWriter for JSON output.

Writing structured JSON output to stderr (via cmd.ErrWriter) is unconventional—typically stdout is used for data and stderr for errors. This affects CLI composability (piping violations to other tools). If violations should be machine-readable output, consider using cmd.Writer (stdout) instead.

🤖 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/root.go` at line 68, The JSON encoder is currently writing to
cmd.ErrWriter (stderr) which is unconventional for machine-readable output;
change the target of json.NewEncoder from cmd.ErrWriter to cmd.Writer so
structured violations are emitted to stdout (or alternatively add a CLI flag to
choose output stream). Locate the line creating the encoder (encoder :=
json.NewEncoder(cmd.ErrWriter)) and replace cmd.ErrWriter with cmd.Writer (or
wire the new flag into the same spot and use the selected io.Writer) to restore
normal CLI composability.
🤖 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/root.go`:
- Around line 51-57: The validator is being constructed with nil OutsiderFinder
and ScopeParser which causes NewValidator to panic; update the code that creates
vld (calling validator.NewValidator with validator.Options) to obtain and pass
real implementations for OutsiderFinder and ScopeParser from the dependency
container or factory instead of nil (e.g., use the container/getter you have in
cmd/root.go to retrieve the OutsiderFinder and ScopeParser instances) so
Options.OutsiderFinder and Options.ScopeParser are non-nil when calling
NewValidator; ensure the fields Git/Logger/SHALength remain set as before and
pass the retrieved instances into the Options struct.

---

Nitpick comments:
In `@cmd/root.go`:
- Line 68: The JSON encoder is currently writing to cmd.ErrWriter (stderr) which
is unconventional for machine-readable output; change the target of
json.NewEncoder from cmd.ErrWriter to cmd.Writer so structured violations are
emitted to stdout (or alternatively add a CLI flag to choose output stream).
Locate the line creating the encoder (encoder := json.NewEncoder(cmd.ErrWriter))
and replace cmd.ErrWriter with cmd.Writer (or wire the new flag into the same
spot and use the selected io.Writer) to restore normal CLI composability.

In `@pkg/validator/git.go`:
- Around line 52-60: Fix the grammatical error in the inline nolint comment in
the gitCommand function: change the comment "//nolint:gosec // no another way"
to use correct wording ("no other way") so it reads e.g. "//nolint:gosec // no
other way" in the DefaultGit.gitCommand function to keep the nolint rationale
grammatically correct.
🪄 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: a48a26eb-17c9-45ae-a5b0-5c16142f946e

📥 Commits

Reviewing files that changed from the base of the PR and between 0bf4765 and ef917fd.

📒 Files selected for processing (6)
  • cmd/root.go
  • pkg/validator/git.go
  • pkg/validator/git_test.go
  • pkg/validator/types.go
  • pkg/validator/validator.go
  • pkg/validator/validator_test.go
💤 Files with no reviewable changes (1)
  • pkg/validator/types.go

Comment thread cmd/root.go
@thumbrise thumbrise merged commit 646f84c into main May 27, 2026
4 checks passed
@thumbrise thumbrise deleted the git branch May 27, 2026 22:58
@github-actions
Copy link
Copy Markdown

🎉 This PR is included in version 0.4.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant