Repository navigation
Conversation
Preview `brew cleanup --dry-run` through the command centre so the Cleanup tab lists unneeded dependencies, old versions, stale downloads and logs with Homebrew's own sizes and total. Clean Up runs `brew cleanup` with the same flags after a confirmation, and an option adds `--prune=all` to both. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Drop the unused raw output from CleanupReport, share the card chrome and error text within the view, derive the row id from its path and give the cleanup fixture its own scenario arm. Co-Authored-By: Claude Opus 5.5 <[email protected]>
📝 Summary
Priority: ➖ Normal Merge Risk: 🔵 Low · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Description check |  |
The description clearly explains the Cleanup tab, preview and execution behaviour, validation, scope, and known test limitations. It is directly related to the changeset. |
| Title check | |
The title, "Add a Cleanup tab for brew cleanup", is concise, specific, and accurately summarises the primary change. |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
![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
🧹 Nitpick comments: 3
Sources/BrewFeatureCleanup/Views/CleanupView.swift: 2
94-94: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExpose one ViewModel property for the disabled state of the controls.
Both the "Check Again" button and the prune-all toggle compose
viewModel.isRefreshing || viewModel.isCleaningUpin the view. The two sites must stay in step. The repository guideline requires one ViewModel property for each UI decision.Add a derived property to
CleanupViewModeland bind to it.Proposed fix
// CleanupViewModel.swift + /// Header controls wait while a preview or a cleanup run is in progress. + var areHeaderControlsDisabled: Bool { + isRefreshing || isCleaningUp + }// CleanupView.swift, headerControls - .disabled(viewModel.isRefreshing || viewModel.isCleaningUp) + .disabled(viewModel.areHeaderControlsDisabled)// CleanupView.swift, pruneAllToggle - .disabled(viewModel.isRefreshing || viewModel.isCleaningUp) + .disabled(viewModel.areHeaderControlsDisabled)As per coding guidelines: "Expose one ViewModel property for each UI decision instead of composing multiple raw flags in a view."
Also applies to: 110-110
Source: Coding guidelines
140-140: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the fixed font size with a theme token.
.font(.system(size: 44))uses a fixed size. It does not scale with Dynamic Type. It is also a magic number in a view. The design system requires relative SwiftUI font styles. The coding conventions require the theme for typography.Use a semantic font token from
BrewUIComponents, or add one for large empty-state icons.As per coding guidelines: "Support Dynamic Type – Yes – use relative SwiftUI font styles, not fixed sizes" and "Extend semantic tokens … instead of introducing raw colours or magic numbers in views."
Source: Coding guidelines
Tests/BrewFeatureCleanupTests/CleanupViewModelTests.swift: 1
153-164: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winWait for cleanup completion, not scheduler yields.
The tests wait for
!viewModel.isCleaningUp, butwaitUntilonly performs 500Task.yield()calls. The cleanup task can still be awaiting the command centre or the repository reload when those iterations finish. Use a target-shared readiness helper or an explicit test completion signal. Do not rely on the phase stream alone because the view model reloads before it clearsisCleaningUp.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
- Source mode: Auto
- Repository: Homebrew/BrewUI
- Branch: main
- File: .coderabbit.config.ts
- Commit: e3510ee
- Layers: Repository TS config (base), Organization UI (inherited)
- Review profile: CHILL
- Plan: Advanced
- Run ID:
1d009767-a908-49a6-a1e7-11c08b0a7557
📒 Files selected for processing: 41
ARCHITECTURE.mdBrewUITests/Fixtures/ScenarioFixtures.swiftBrewUITests/Harness/BrewUITestScenario.swiftBrewUITests/Screens/CleanupScreen.swiftBrewUITests/Screens/Sidebar.swiftBrewUITests/Tests/CleanupUITests.swiftBrewUITests/Tests/NavigationUITests.swiftHomebrew.xcodeproj/project.pbxprojHomebrew/BrewApp.swiftHomebrew/Features/MainWindow/Views/MainWindowView.swiftHomebrew/Localizable.xcstringsHomebrew/Views/MainSidebarView.swiftHomebrew/Views/SidebarItem.swiftPackage.swiftREADME.mdSources/BrewAccessibilityID/AXID.swiftSources/BrewAppEnvironment/RepositoryEnvironment.swiftSources/BrewAppEnvironment/UnimplementedRepositories.swiftSources/BrewCLI/LiveBrewMutatingCommandFactory.swiftSources/BrewCore/Models/CleanupReport+Placeholdable.swiftSources/BrewCore/Models/CleanupReport.swiftSources/BrewCore/Operations/BrewCommands.swiftSources/BrewCore/Operations/BrewMutatingCommandFactory.swiftSources/BrewCore/Operations/BrewOperationModels.swiftSources/BrewFeatureCleanup/Resources/Localizable.xcstringsSources/BrewFeatureCleanup/ViewModels/CleanupRemovalItem.swiftSources/BrewFeatureCleanup/ViewModels/CleanupViewModel.swiftSources/BrewFeatureCleanup/Views/CleanupRoot.swiftSources/BrewFeatureCleanup/Views/CleanupView.swiftSources/BrewRepositories/BrewCleanupRepository.swiftSources/BrewRepositories/CleanupOutputParser.swiftSources/BrewRepositoryInterfaces/CommandJob.swiftSources/BrewRepositoryInterfaces/Fakes/Stubs.swiftSources/BrewRepositoryInterfaces/PreviewSupport/PreviewSupport+Cleanup.swiftSources/BrewRepositoryInterfaces/Protocols/CleanupRepository.swiftTests/BrewAccessibilityIDTests/AXIDTests.swiftTests/BrewCoreTests/BrewCommandsTests.swiftTests/BrewCoreTests/BrewOperationIDTests.swiftTests/BrewFeatureCleanupTests/CleanupViewModelTests.swiftTests/BrewRepositoriesTests/BrewCleanupRepositoryTests.swiftTests/BrewRepositoriesTests/CleanupOutputParserTests.swift
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.
MikeMcQuaid
left a comment
MikeMcQuaid
left a comment
There was a problem hiding this comment.
brew cleanup is run automatically. Why does it need dedicated UI?
- Clear a stale refresh failure when the preview switches option. - Expose one view model property for disabling the header controls. - Wait on a deadline in the view model tests rather than a yield count. Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
@coderabbitai Nitpicks, addressed in 2f2b5a7:
|
|
@MikeMcQuaid Fair point — automatic cleanup covers most of it. What it doesn't give a GUI user:
If you'd rather not have a dedicated tab, I'm happy to shrink it to a "Clean Up" action inside Doctor or wherever you see best fit. |
There was a problem hiding this comment.
🧹 Nitpick comments: 1
Tests/BrewFeatureCleanupTests/CleanupViewModelTests.swift: 1
158-158: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftReplace the wall-clock deadline with a readiness signal.
If command-centre execution or the subsequent reload takes more than five seconds on a busy test worker,
waitUntilrecords a failure even when both complete correctly. Wait for an explicit completion signal from the test double. Use an injected clock if the test must check timeout behaviour. As per coding guidelines, “Drive timeout tests from explicit readiness signals and injected clocks rather than short wall-clock deadlines.”Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
- Source mode: Auto
- Repository: Homebrew/BrewUI
- Branch: main
- File: .coderabbit.config.ts
- Commit: e3510ee
- Layers: Repository TS config (base), Organization UI (inherited)
- Review profile: CHILL
- Plan: Advanced
- Run ID:
b768f31c-a0a1-4acf-8780-ccf503559130
📒 Files selected for processing: 5
Sources/BrewFeatureCleanup/ViewModels/CleanupViewModel.swiftSources/BrewFeatureCleanup/Views/CleanupView.swiftSources/BrewRepositories/BrewCleanupRepository.swiftTests/BrewFeatureCleanupTests/CleanupViewModelTests.swiftTests/BrewRepositoriesTests/BrewCleanupRepositoryTests.swift
🚧 Files skipped from review as they are similar to previous changes: 1
- Tests/BrewRepositoriesTests/BrewCleanupRepositoryTests.swift
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.
|
CodeRabbit chat interactions are restricted to organization members for this repository. Ask an organization member to interact with CodeRabbit, or set |
Co-Authored-By: Claude Opus 5.5 <[email protected]>
MikeMcQuaid
left a comment
MikeMcQuaid
left a comment
There was a problem hiding this comment.
Please don't request my rereview. Want @graeme's thoughts before we go further here.
What and why
Adds a Cleanup tab for
brew cleanup, the main maintenance task missing from BrewUI. Old versions, stale downloads and unneeded dependencies build up silently, and Cakebrew users switching to BrewUI expect a place to reclaim that space without Terminal.brew cleanup --dry-runthrough the command centre, so the preview appears in the console like Doctor's read. Installs and upgrades elsewhere change what is stale, so the preview is not cached across visits.brew cleanupwith the same flags as the preview, so what runs is what was reviewed, and previews again afterwards. It is a mutating operation, so the installed inventory reconciles afterwards.--prune=allto both the preview and the run.brew cleanupalready runs autoremove unlessHOMEBREW_NO_AUTOREMOVEis set, so there is no separatebrew autoremoveaction.Out of scope:
brew bundle cleanup. It uninstalls everything a chosen Brewfile does not list, so it needs a file picker and deserves its own review.Validation
macOS 27.0.1, Apple M5 Pro, Xcode 27.1, Swift 6.4.
scripts/test: all pass, including the new parser, repository, view model, command and AXID tests.mint run swiftformat --lint .,mint run swiftlint lint --strict, BrewUILint over the production tree andscripts/localize verify: all pass.scripts/test-ui -only-testing:BrewUITests/CleanupUITests -only-testing:BrewUITests/NavigationUITests: 5 tests pass.brew cleanup --dry-runprints in Terminal, and the dry run streamed in the console. Clean Up opened the confirmation, and Cancel left everything in place: a later dry run still reported 67.5MB.scripts/localize syncstops on Xcode 27.1, which names package build folders<Target>-t.build, but the script looks for<Target>.build. Its app-catalog pass also reorders existing translations. I synced the new catalog withxcstringstooldirectly and added the two app-catalog keys by hand. The app catalog matches what the tool produces, ignoring order.Screenshots
Confirmation before running
brew cleanup:Both builds ran against real Homebrew 7.0.10 on the same Mac, in light appearance.
AI assistance
Claude Code (Claude Opus 5.5) wrote the implementation, tests and this description, and ran the checks listed above.
🤖 Generated with Claude Code