refactor: codebase cleanup — dead accessors, unused strings, stale docs - #1517
Merged
Merged
Conversation
Swap.freePercent, ServersNotifier.isAutoRefreshOn, TerminalSession.isBackendClosed and Battery.isLiPoly are referenced nowhere in lib, test or integration_test. Battery.isLiPoly lost its last reader when battery parsing moved into sbm_parser, so its Rust counterpart's doc no longer points at a Dart method.
No caller inside or outside the crate; FsRoots::check_within already answers the same question for the only place that needs it.
benchmarkBlockSize, benchmarkCompare, benchmarkCompareEmpty, benchmarkDiedUnreported, benchmarkLinuxOnly, dockerImagesFmt, monitorNoAgent, scheduledTaskScheduleHint, serverDetailOrder, serviceEnabled, serviceSelectHint and trayTitle are referenced nowhere in lib, test or integration_test. Removed from every locale and regenerated; nothing looks them up by name.
RETIREMENT.md addressed every shim by file and line number, and every one of those line numbers had drifted since it was written, so none of them resolved to the code it described. Name the file and symbol instead, which cannot go stale. structure.md listed a model/sftp/ directory that does not exist (the models live under file/) and omitted model/ai/ and data/service/.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Essentials Run ID: ⛔ Files ignored due to path filters (16)
📒 Files selected for processing (25)
💤 Files with no reviewable changes (6)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe change replaces retirement references with stable file-and-symbol locations. It updates English and Chinese structure documentation. It removes unused public getters and a Rust path helper. It removes obsolete localization entries across supported locale files. It also updates battery parser documentation without changing runtime behavior. Suggested reviewers: Priority: ⬇️ Low Change: Refactor 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Deploying serverbox with
|
| Latest commit: |
cdf4c91
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://23cdcf1b.serverbox.pages.dev |
| Branch Preview URL: | https://chore-code-health-followup.serverbox.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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.
Summary
A code-health pass over the first-party tree (Dart app, Rust workspace, monitor agent, docs). It removes the dead code and stale documentation the sweep could prove, and leaves everything where the evidence was not conclusive.
Nothing here changes behaviour, storage, schema, protocols, FFI contracts or platform configuration.
What changed
refactor: remove unused Dart accessorsSwap.freePercent,ServersNotifier.isAutoRefreshOn,TerminalSession.isBackendClosedandBattery.isLiPolyare referenced nowhere inlib,testorintegration_test.Battery.isLiPolylost its last Dart reader when battery parsing moved tosbm_parser, so the Rust method's doc comment no longer points at it.refactor(monitor): remove unused fs_roots::is_withinFsRoots::check_withinalready answers the same question for the only place that needs it. The now-unusedPathimport goes with it.chore(l10n): drop twelve unused stringsbenchmarkBlockSize,benchmarkCompare,benchmarkCompareEmpty,benchmarkDiedUnreported,benchmarkLinuxOnly,dockerImagesFmt,monitorNoAgent,scheduledTaskScheduleHint,serverDetailOrder,serviceEnabled,serviceSelectHint,trayTitlehad no reference in any Dart source. Removed from all 16 locales and regenerated withflutter gen-l10n. This follows the precedent already recorded inRETIREMENT.md.docs: correct drifted structure and retirement referencesRETIREMENT.mdaddressed every shim byfile:line, and every one of those line numbers had drifted since it was written. It now names the file and symbol instead.structure.md(en and zh) listed amodel/sftp/directory that does not exist, and omittedmodel/ai/anddata/service/.What the audit checked and left alone
@visibleForTestingsymbol is also called from production (parsers, command builders, payload builders, seam interfaces). No test-only hook was found to delete, so no test exposure needed to change.lib/**file is imported, every public type is referenced, and the only unreferenced declarations were the four accessors above.updateShouldNotify,didChangeLocales,onWindowClose,updateByte,saveToFile,saveSyncCheckpoint) or enum constants reached through.values— all kept.skip: trueis the project's opt-in e2e gate), duplicated helpers or unused helpers.dart analyze,cargo clippy --workspace --all-targets, the panel'seslint --max-warnings 0andsvelte-checkare all clean. Reviewed the TLS opt-ins (badCertificateCallbackis gated behind the per-serverallowInsecure/ignoreCert), shell quoting/injection boundaries, the memory/swap zero-total paths (all call sites guardtotal > 0) and the AI command-fencing path. No change was warranted.Validation
dart analyze lib test integration_test— no issues.cargo check -p server_box_monitor --locked --offlineandcargo test -p server_box_monitor --test fs_roots— 12 passed.cargo test -p sbm_parser --locked --offline— passes (e2e cases remain opt-in and ignored).flutter test --no-pub --timeout 30son the suites covering the touched files (terminal_session, server detail metrics, server status update, server save/selection, server edit logic,l10n_fr,app_locale) — 63 passed.flutter gen-l10nandnode docs/scripts/check-locale-parity.mjs— OK, 32/32 pages.Summary by CodeRabbit
Documentation
Maintenance