Migrate lfs-tidy onto the CleanPipeline seam#205
Merged
Conversation
Replace the hand-rolled clean loop in git-lfs-tidy with the shared `git_tidy_core::clean::run_clean` seam (issue #118 step 3). lfs groups items by repo, so we call the shared loop once per group over that group's items, accumulating each group's `CleanResult` into one aggregate `succeeded`/`failed`/`skipped`. A new `should_clean` predicate is the pure `decide` filter: it admits only orphaned objects when `--prune` is set, so every other classification (untracked, missing, healthy) is rejected and counted as a skip by the pipeline, exactly matching the old `skipped += 1; continue;`. The `act` closure owns everything lfs-specific — count/byte extraction, the dry-run-vs-real branch, the "would prune"/"pruned"/"error: could not prune" wording, the `git.lfs_prune` call, and the `PrunedRepo` success record (dry-run still returns `Outcome::Cleaned` with `dry_run: true`). The per-group health recommendations are the one aggregate extra the pipeline doesn't model, so they stay tool-side around the per-group `run_clean` call. We detect `has_untracked`/`has_missing` directly from the group's item classifications and emit the recommendation lines after that group's `run_clean` call, pushing each into `recommendations`. Because each group is processed in full — prune lines first, then recommendation lines — before the next group, output ordering is preserved exactly. The `LfsCleanResult` wrapper (which derefs to `CleanResult<PrunedRepo>` and carries the `#[allow(dead_code)]` `recommendations`) is kept unchanged, so the public signature is stable and every existing unit and integration test passes without edits. Refs #118
Doc-only follow-ups surfaced by review. Update the CleanPipeline-seam description in CLAUDE.md from "migrated so far: git-repo-tidy" to reflect that all seven tools now sit on the shared run_clean seam, noting the shared should_clean_landed filter for branch/worktree, the per-group call pattern for grouped tools (with stash's descending pre-sort), and lfs-tidy's tool-side recommendations. Drop the stale "candidate to drop once lfs moves onto the shared CleanPipeline" line from LfsCleanResult.recommendations, since lfs is now on the seam, and describe how the field is computed tool-side instead. Refs #118
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Step 3 of #118 (5 of 5). Moves
git-lfs-tidyontogit_tidy_core::clean::run_cleanwhile keeping itsLfsCleanResultwrapper.decideadmits only orphaned objects when--pruneis set; every other classification is counted skipped, matching the old match arms.actowns count/byte extraction, dry-run branching, the prune call, and thePrunedReporecord. Per-group health recommendations are the aggregate extra the pipeline doesn't model, so they stay tool-side: detected from the group's classifications and emitted after that group's prune lines, keeping output ordering exact.Behavior-preserving: all existing tests (incl.
recommendationsassertions) pass unchanged. Local CI green.Refs #118