Remove the clean subcommand from v1.0.0#35
Conversation
`clean` was the only destructive subcommand, the only one carrying open correctness bugs, and the only one whose `--apply` path had never run in CI. Ship 1.0.0 without it rather than ship a half-tested destroyer. Removes `cmd_clean`, its dispatch arm, its `usage` and header-comment entries, the README command section, and the "Report before destroy" constraint. CI now asserts `clean` is an unknown command and that `help` no longer advertises it. `AGENTS.md` regression 2 (`git branch --merged` flags branches with no commits of their own) is kept and annotated: no code relies on it today, but losing the note is how the bug returns when `clean` does. The README gains a "Removing worktrees" section with the by-hand equivalent. Closes #17. The `TREES_RM` design and the full `clean` bug history are preserved in #34, which tracks bringing the command back. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe ChangesClean command removal
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 @.github/workflows/ci.yml:
- Around line 89-94: Update the CI checks for the script invocation around the
`clean` command to capture its stderr, require a nonzero exit status, and assert
that the output contains the expected unknown-command diagnostic rather than
accepting any failure. Also tighten the help-output grep to match `clean` as a
whole word while preserving the existing assertion that it is not advertised.
🪄 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: a4f9a4b2-c388-47df-bfa3-ea1e0c4ce3e0
📒 Files selected for processing (4)
.github/workflows/ci.ymlAGENTS.mdREADME.mdgit-trees
💤 Files with no reviewable changes (1)
- git-trees
| # `clean` was removed before v1.0.0 (#17); it must not be dispatchable. | ||
| if bash "$T" clean >/dev/null 2>&1; then | ||
| echo "FAIL: clean should be an unknown command"; exit 1 | ||
| fi | ||
| if bash "$T" help 2>&1 | grep -q clean; then | ||
| echo "FAIL: help still advertises clean"; exit 1 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert that clean is unknown, not merely unsuccessful.
The current check passes for any nonzero exit, including a recognized clean command that fails for an unrelated reason. Capture stderr and assert the expected unknown-command diagnostic; retain the nonzero-status check as well. Consider matching clean as a whole word in help output.
🤖 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 @.github/workflows/ci.yml around lines 89 - 94, Update the CI checks for the
script invocation around the `clean` command to capture its stderr, require a
nonzero exit status, and assert that the output contains the expected
unknown-command diagnostic rather than accepting any failure. Also tighten the
help-output grep to match `clean` as a whole word while preserving the existing
assertion that it is not advertised.
| ## Removing worktrees | ||
|
|
||
| Reports by default; `--apply` executes. Three passes: | ||
| There is no `clean` subcommand in 1.0.0. Remove a worktree and its branch by | ||
| hand: | ||
|
|
||
| - **gone** — branches whose upstream is deleted (`[gone]`) | ||
| - **merged** — branches merged into the default branch, *excluding* those with | ||
| no commits of their own (a branch freshly cut from `main` is technically | ||
| merged but isn't finished work) | ||
| - **older-than** — worktrees whose directory mtime exceeds N days | ||
| ```bash | ||
| git worktree remove <path> | ||
| git branch -d <branch> # -d refuses unmerged work; escalate to -D yourself | ||
| git worktree prune | ||
| ``` | ||
|
|
||
| `--gone` and `--merged` are mutually exclusive selectors; passing neither runs | ||
| both. Uses `git branch -d` (safe) and tells you when to escalate to `-D`. Never | ||
| removes the repo root or your current directory. | ||
| A `clean` command shipped in pre-release versions and was pulled before 1.0.0 — | ||
| it was the only destructive subcommand, it carried open correctness bugs, and its | ||
| `--apply` path had never run in CI. It will return once it is fully tested and | ||
| its removal step is routable to a trash can rather than being unrecoverable. See | ||
| [the reintroduction issue](https://github.com/brightdigit/git-trees/issues/34). |
There was a problem hiding this comment.
we don't need to mention that there was a subcommand. This will be for the first release
1.0.0 is the first release, so the README has no prior version to refer back to. The paragraph explaining that a `clean` command once existed and why it was pulled reads as history to a reader who has none. Removed, along with the "No `clean` subcommand" phrasing in Known limitations, which implied one was expected. The manual `git worktree remove` recipe stays — that part is useful. The rationale is not lost: it lives in AGENTS.md and issue #34. CI only checked that `clean` exited nonzero, which a dispatched-but-broken `clean` would also satisfy. Assert the "unknown command 'clean'" diagnostic instead, and match `clean` as a whole word in the help output so the legitimate "clean/dirty" text cannot trip it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Agreed — dropped in 2ff08f3. The "A Same archaeology was in PR #40's CHANGELOG at two spots; cleaned up there too, along with the now-orphaned Also took the CodeRabbit point on |
1 participant
Merge order: 1 of 6, into
v1.0.0-integration. Merge this first — it rewritesregions of
git-trees,README.md,AGENTS.md, andci.ymlthat the otherfive PRs also touch.
Closes #17.
What
cleanis the only destructive subcommand, the only one carrying opencorrectness bugs, and the only one whose destructive path has never run in CI.
Per #17, it is pulled from v1.0.0 rather than fixed under tag pressure.
git-treescmd_cleanand its section, theclean)dispatch arm, theusageentry, the header-comment lineREADME.mdAGENTS.mdcleantest-checklist item.github/workflows/ci.ymlcleanis unknown and unadvertised_default_branchand_path_forstay —addandliststill use them.The regression note stays
AGENTS.mdregression 2 (git branch --mergedflags branches with no commitsof their own) is kept and annotated. No code relies on it now, but it is the
reason the
rev-list --countguard has to come back withclean, and losingthe note is how the bug returns.
Follow-up
#34 carries #17's Part 2 (
TREES_RMconfigurable removal command) and Part 3(all six historical
cleanbugs) plus acceptance criteria. Nothing there isimplementable while the command is absent, so it tracks the reintroduction.
Verified
bash -n git-treesandshellcheck -s bash git-treescleanfile://fixture:git trees cleanexits 1 with an unknown-commanderror;
helphas zerocleanmentions;list --jsonstill parsesAGENTS.mdregressions re-checked:add feature-x→ upstreamorigin/feature-x;add brandnew→ upstreamorigin/brandnew, notorigin/maingrep -rn clean README.md AGENTS.md git-treesreturns only intentional uses(the
clean/dirtystate inlist, the new docs section, the Reintroducecleanwith a configurable removal command #34 pointers)Conflicts to expect
None with PR 2 (disjoint hunks). PR 4 edits the README quickstart this PR also
touches; PR 5 rewrites the
ci.ymlsmoke block.🤖 Generated with Claude Code
Summary by CodeRabbit
Breaking Changes
git trees cleancommand from the CLI.cleanis no longer listed in help or recognized as a valid command.Documentation
Tests
cleanis rejected and omitted from help output.