Skip to content

Conversation

@adeeshperera
Copy link

@adeeshperera adeeshperera commented Oct 10, 2025

  • Return descriptive errors when git commands fail instead of silently continuing with empty statistics
  • Prevent misleading "no changes" display when git operations encounter issues
  • Update tests to expect errors for invalid repository scenarios

Summary by CodeRabbit

  • Bug Fixes

    • Improved error reporting when retrieving staged, unstaged, and untracked files, ensuring failures are surfaced immediately.
    • Enhanced reliability when collecting line-change statistics; operations now stop with a clear error if retrieval fails.
    • For invalid or non-repository directories, file statistics now return explicit errors instead of ambiguous results.
  • Refactor

    • Streamlined control flow to handle errors first, then process results, improving robustness.
  • Tests

    • Updated tests to expect errors and no results for invalid and non-repository directories.

adeeshperera and others added 2 commits October 10, 2025 22:10
- Return descriptive errors when git commands fail instead of silently continuing with empty statistics
- Prevent misleading "no changes" display when git operations encounter issues
- Update tests to expect errors for invalid repository scenarios
fix DFanso#84  : properly handle git command errors in GetFileStatistics
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 10, 2025

Walkthrough

Refactors internal/stats/statistics.go to propagate errors immediately when fetching staged, unstaged, untracked files and collecting line stats, then conditionally process outputs. Updates tests in internal/stats/statistics_test.go to expect errors and nil stats for invalid or non-git directories.

Changes

Cohort / File(s) Summary
Error handling refactor in stats implementation
internal/stats/statistics.go
Replaces combined error/data checks with early error returns for staged, unstaged, and untracked retrieval and line stat collection; processes outputs only if present after error checks.
Tests aligned to new error propagation
internal/stats/statistics_test.go
Adjusts expectations: GetFileStatistics returns an error and nil stats for non-existent and non-git directories.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Caller
  participant Stats as GetFileStatistics
  participant Git as GitHelpers

  Caller->>Stats: GetFileStatistics(path)
  Stats->>Git: GetStagedFiles(path)
  alt error on staged
    Git-->>Stats: error
    Stats-->>Caller: error
  else ok
    Git-->>Stats: stagedOutput
    Stats->>Git: GetUnstagedFiles(path)
    alt error on unstaged
      Git-->>Stats: error
      Stats-->>Caller: error
    else ok
      Git-->>Stats: unstagedOutput
      Stats->>Git: GetUntrackedFiles(path)
      alt error on untracked
        Git-->>Stats: error
        Stats-->>Caller: error
      else ok
        Git-->>Stats: untrackedOutput
        Stats->>Git: GetStagedLineStats(path)
        alt error on line stats
          Git-->>Stats: error
          Stats-->>Caller: error
        else ok
          Git-->>Stats: lineStatsOutput
          Stats-->>Caller: aggregated stats
        end
      end
    end
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I nibbled on errors, crisp as fall air,
Hopped past the hedges of nested beware.
Now, if trouble sprouts, I squeak it upfront—
Then munch the output, bite by bite, no stunt.
Git burrow tidy, stats carrot-bright—
Quick checks, clean hops, all set right. 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly summarizes the main change by specifying that git command errors are now handled properly in GetFileStatistics and includes the issue reference for context without introducing ambiguity.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 525f493 and 61fa6d8.

📒 Files selected for processing (2)
  • internal/stats/statistics.go (2 hunks)
  • internal/stats/statistics_test.go (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
internal/stats/statistics_test.go (1)
internal/stats/statistics.go (1)
  • GetFileStatistics (14-86)
⏰ 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: Build Go Binary (windows-latest)
🔇 Additional comments (7)
internal/stats/statistics.go (5)

24-29: LGTM! Error handling for staged files is correct.

The pattern of checking for errors first, returning immediately with a descriptive error message, and then processing output only when present is clean and correct.


34-39: LGTM! Error handling for unstaged files is correct.

Consistent with the staged files pattern—error checking is performed first, followed by conditional output processing.


44-49: LGTM! Error handling for untracked files is correct.

The error handling pattern is consistently applied across all file status queries.


62-65: LGTM! Error handling for line statistics is correct.

The error is properly propagated before attempting to process the output, maintaining consistency with the earlier error handling patterns.


14-86: Error propagation is properly handled by all callers Verified that the sole invocation in cmd/cli/createMsg.go checks the returned error and exits on failure.

internal/stats/statistics_test.go (2)

27-34: LGTM! Test correctly expects error for non-existent directory.

The test has been properly updated to expect an error when GetFileStatistics is called with a non-existent path, and verifies that stats is nil when an error occurs.


45-52: LGTM! Test correctly expects error for non-git directory.

The test has been appropriately updated to expect an error when GetFileStatistics is called with a non-git directory, and verifies that stats is nil when an error occurs.


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

Copy link
Owner

@DFanso DFanso left a comment

Choose a reason for hiding this comment

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

LGTM 🎊

@DFanso DFanso added bug Something isn't working enhancement New feature or request hacktoberfest Eligible for Hacktoberfest hacktoberfest-accepted Approved Hacktoberfest contribution go Pull requests that update go code labels Oct 10, 2025
@DFanso DFanso linked an issue Oct 10, 2025 that may be closed by this pull request
@DFanso DFanso merged commit 19c7279 into DFanso:main Oct 10, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request go Pull requests that update go code hacktoberfest Eligible for Hacktoberfest hacktoberfest-accepted Approved Hacktoberfest contribution

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Missing Error Handling for Empty File Lists

2 participants