Skip to content

Conversation

@2bndy5
Copy link
Contributor

@2bndy5 2bndy5 commented Feb 6, 2026

In porting #180 to rust (cpp-linter/cpp-linter-rs#260), I found a few spots for improvement.
These changes should be more performant when there are staged changes.

Summary by CodeRabbit

  • Chores

    • Normalized JSON line endings across the repo.
    • Suppressed a doc import type-check warning.
  • Bug Fixes

    • Improved detection and handling of staged changes and diff computation for more accurate change reporting.
  • Tests

    • Marked a test to run without clang-related tooling.

@2bndy5 2bndy5 added the enhancement New feature or request label Feb 6, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 6, 2026

Walkthrough

Updates Git diff computation and staged-status handling in the git module; adds JSON LF normalization in .gitattributes; suppresses type-check for docutils in docs config; and marks a test to run without clang tooling.

Changes

Cohort / File(s) Summary
Git module logic
cpp_linter/git/__init__.py
Reworked staged-status into a single bitmask; simplified use_index check to any(status & STAGED_STATUS ...); obtain base via .peel(Commit); adjust diff paths for index vs non-index, rename diff labels accordingly; call legacy_parse_diff(str(diff_obj)) on GitError.
Configuration & docs
.gitattributes, docs/conf.py
Added *.json text eol=lf to .gitattributes; added # type: ignore[import-untyped] to docutils import in docs/conf.py; minor doc wording change in diff parser note.
Tests
tests/capture_tools_output/test_tools_output.py
Added @pytest.mark.no_clang marker to test_get_sha to control test selection.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

minor

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Title check ⚠️ Warning The PR title claims optimization of get_diff(), but the primary changes affect get_sha() and staged status detection in the git module. The title is misleading about the main focus. Update the title to accurately reflect that optimizations target get_sha() and staged change detection, e.g., 'perf: optimize get_sha() with bitwise status comparison'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch optimize-get-sha

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.

coderabbitai[bot]

This comment was marked as resolved.

@2bndy5

This comment was marked as resolved.

@2bndy5 2bndy5 marked this pull request as draft February 6, 2026 07:43
@2bndy5

This comment was marked as outdated.

@codecov
Copy link

codecov bot commented Feb 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.29%. Comparing base (9a5897a) to head (b6fa83e).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #185      +/-   ##
==========================================
- Coverage   98.30%   98.29%   -0.01%     
==========================================
  Files          24       24              
  Lines        1942     1938       -4     
==========================================
- Hits         1909     1905       -4     
  Misses         33       33              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

In porting #180 to rust (cpp-linter/cpp-linter-rs#260),
I found a few spots for improvement.
These changes should be more performant when
there are staged changes.
@2bndy5 2bndy5 marked this pull request as ready for review February 6, 2026 09:15
@2bndy5 2bndy5 changed the title perf: small optimizations to get_sha() perf: small optimizations to get_diff() Feb 6, 2026
@2bndy5 2bndy5 merged commit 7659996 into main Feb 6, 2026
43 checks passed
@2bndy5 2bndy5 deleted the optimize-get-sha branch February 6, 2026 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant