Skip to content

Skip commits with a Drift: ignore trailer - #4

Merged
gmr merged 1 commit into
gmr:mainfrom
dave-shawley:feature/ignore-drift-trailer
Aug 20, 2026
Merged

Skip commits with a Drift: ignore trailer#4
gmr merged 1 commit into
gmr:mainfrom
dave-shawley:feature/ignore-drift-trailer

Conversation

@dave-shawley

@dave-shawley dave-shawley commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

analyze() now excludes commits from the drift report when their message carries a Drift: ignore trailer, letting an author explicitly vouch for a change instead of it showing up as unexplained drift.

Problem

Some commits (e.g. a reformat, a deliberate one-off tweak) touch paths that drift would otherwise report, even though the author already knows about them and doesn't want them flagged. There was no way to mark a commit as intentionally exempt.

Solution

  • A non-merge commit whose message has a Drift: ignore trailer is traversed but never classified, the same treatment merges already get.
  • The trailer's token and value are matched ASCII case-insensitively, since gix only trims whitespace on trailers and doesn't fold case.
  • Added ignored_commits_skipped to the JSON report, counted separately from commits_scanned and merge_commits_skipped.
  • Documented the new field and trailer semantics in README's "Defined behavior" section, per the project's contract that semantic changes update that section in the same commit.

Testing

  • cargo test, cargo clippy --all-targets -- -D warnings, and cargo fmt --check all pass.
  • Added CLI tests: log-mode exclusion, tree-mode leaving an unattributed path, and case-insensitive trailer matching.

Summary by CodeRabbit

  • New Features
    • Commits marked with a Drift: ignore trailer are now excluded from drift analysis.
    • Ignored commits are counted separately in reports.
    • Trailer matching is case-insensitive.
    • Tree reports identify paths changed only by ignored commits as unattributed.
  • Documentation
    • Added report field details and clarified how ignored commits affect scanning and attribution.

- analyze() now excludes non-merge commits carrying a Drift: ignore
  trailer, the same way merges are traversed but never classified
- token and value are matched ASCII case-insensitively since gix only
  trims whitespace on trailers, it doesn't fold case
- add ignored_commits_skipped to the report, counted separately from
  commits_scanned and merge_commits_skipped
- document the new field and trailer semantics in README's Defined
  behavior section
- add cli tests covering log mode exclusion, tree mode leaving an
  unattributed path, and case-insensitive matching

Co-authored-by: Claude <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 20, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b44a2dc4-227d-47e8-8391-701113a23253

📥 Commits

Reviewing files that changed from the base of the PR and between 3dd503e and 4f5de55.

📒 Files selected for processing (4)
  • README.md
  • src/report.rs
  • src/scan.rs
  • tests/cli.rs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The scanner now detects case-insensitive Drift: ignore trailers, skips those commits from classification, counts them separately, and reports their tree-mode paths as unattributed. The report schema, tests, and README document this behavior.

Changes

Ignored commit handling

Layer / File(s) Summary
Report contract
src/report.rs
Summary and Report now include ignored_commits_skipped. Report construction and fixtures propagate the value.
Scan ignore-trailer handling
src/scan.rs
The scanner detects matching trailers, skips affected non-merge commits, and includes the count in the report.
Behavior validation and documentation
tests/cli.rs, README.md
Tests cover exclusion, case-insensitive matching, and tree-mode unattributed paths. The README documents the field and behavior.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 4f5de

The change adds an explicit trailer-based way to exclude intentional drift from reports and records those exclusions separately; no actionable merge-blocking risk remains beyond normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant CLI
  participant scan
  participant Commit
  participant Report
  CLI->>scan: start scan
  scan->>Commit: load non-merge commit
  scan->>Commit: read trailers
  Commit-->>scan: return Drift: ignore match
  scan->>Report: count skipped commit
  scan-->>CLI: return report without classified ignored commit
Loading

Possibly related PRs

  • gmr/drift#1: Introduced the scan and report logic extended by this change.

Suggested reviewers: gmr

Poem

I hop through commits in the morning light,
Mark ignored changes and keep counts right.
Paths left unseen stay gently declared,
Reports carry every detail prepared.
— A careful rabbit 🐇

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: skipping commits with a Drift: ignore trailer.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

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.

@dave-shawley

Copy link
Copy Markdown
Contributor Author

🤖 This comment was posted by Claude on behalf of @dave-shawley

Status check for this cycle:

  • CodeRabbit has reviewed HEAD (4f5de551) and reported no actionable comments ("No actionable comments were generated in the recent review."). No review threads are open.
  • The Testing and Docker GitHub Actions workflow runs for this PR are stuck in action_required — GitHub is holding them for maintainer approval since this PR comes from a fork (dave-shawley/drift). I attempted to approve the runs via the API and was correctly denied (403, requires admin rights on gmr/drift), so this needs a maintainer to approve.

@gmr — could you approve the pending workflow runs for this PR (Actions tab, or "Approve and run") so CI can execute? Nothing else is blocking merge once CI runs green.

@gmr
gmr merged commit 5994531 into gmr:main Aug 20, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants