refactor: delete dead code revealed by removing dead_code allows#1052
Merged
joshrotenberg merged 1 commit intoJul 24, 2026
Merged
Conversation
Removed all 106 #[allow(dead_code)] annotations from crates/redisctl/src (42 blanket module-level, 64 item-level), which existed only because the now-deleted lib target used nothing. The lint then flagged 24 genuinely dead items; deleted 20: - Superseded LDAP path: rbac.rs handle_ldap_command and five rbac_impl.rs LDAP functions (the live path is ldap.rs). - Superseded debuginfo handlers handle_debuginfo_all/node/database (the _binary variants are live). - Unused helpers: two ACL detail printers, parse_database_id, the OptimizationResult struct + method, two keyring helpers, build_database_payload, WorkflowResult::failure, ConnectionManager::save_config, and WorkflowContext.wait_timeout (set but never read, so init-cluster already ignored --wait-timeout). Kept with a targeted #[allow(dead_code)] and a comment (see #1049): - error.rs NoProfileConfigured / MissingCredentials / UnsupportedDeploymentType: intended error types with messages and suggestions that no code path constructs yet. To be wired up, not cut. - connection.rs CloudConnectionInfo / EnterpriseConnectionInfo credential fields: populated but not read by the curl builder, so `api ... --curl` omits its auth headers. Likely a real bug; kept pending that fix. Net ~670 lines removed; 5 documented allows remain instead of 106 blanket ones, and the dead-code lint now covers the whole crate. Part of #1049 (CODE-12), phase 2b.
joshrotenberg
added a commit
that referenced
this pull request
Jul 24, 2026
* refactor: drop the redundant redisctl lib target The redisctl crate had both a lib target (lib.rs, declaring every module pub(crate)) and a bin target (main.rs, re-declaring the same modules), so the whole tree compiled twice. The lib exposed no public API and was used by nothing (no test or bench imports redisctl::...); it existed only for a docs.rs page, and lib.rs itself says redisctl is not meant to be used as a library. Delete lib.rs and move its overview into a module doc on main.rs. The crate now compiles once, as a binary. This is the structural half of the lib/bin cleanup; removing the 110 now-pointless #[allow(dead_code)] annotations and deleting the dead code the lint then reveals is the follow-up. Part of #1049 (CODE-12), phase 2a. * refactor: delete dead code revealed by removing dead_code allows (#1052) Removed all 106 #[allow(dead_code)] annotations from crates/redisctl/src (42 blanket module-level, 64 item-level), which existed only because the now-deleted lib target used nothing. The lint then flagged 24 genuinely dead items; deleted 20: - Superseded LDAP path: rbac.rs handle_ldap_command and five rbac_impl.rs LDAP functions (the live path is ldap.rs). - Superseded debuginfo handlers handle_debuginfo_all/node/database (the _binary variants are live). - Unused helpers: two ACL detail printers, parse_database_id, the OptimizationResult struct + method, two keyring helpers, build_database_payload, WorkflowResult::failure, ConnectionManager::save_config, and WorkflowContext.wait_timeout (set but never read, so init-cluster already ignored --wait-timeout). Kept with a targeted #[allow(dead_code)] and a comment (see #1049): - error.rs NoProfileConfigured / MissingCredentials / UnsupportedDeploymentType: intended error types with messages and suggestions that no code path constructs yet. To be wired up, not cut. - connection.rs CloudConnectionInfo / EnterpriseConnectionInfo credential fields: populated but not read by the curl builder, so `api ... --curl` omits its auth headers. Likely a real bug; kept pending that fix. Net ~670 lines removed; 5 documented allows remain instead of 106 blanket ones, and the dead-code lint now covers the whole crate. Part of #1049 (CODE-12), phase 2b. * ci: use per-package test target flags now that redisctl is bin-only Dropping the redisctl lib target broke the test-unit job: it ran cargo test --package <pkg> --lib --bins, and --lib errors with "no library targets found" for redisctl, which now has only a bin. Give each matrix package its own target flags: redisctl-core --lib, redisctl --bins, redisctl-mcp --lib --bins. --bins still runs the tests in main.rs (including the MCP safety-tier registration tests), so ci-status keeps gating them.
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
Phase 2b of the de-complexification (#1049), finding CODE-12. Stacked on #1051 (base is
chore/code-12-drop-lib); review/merge that first.With the lib target gone (#1051), the 106
#[allow(dead_code)]annotations incrates/redisctl/src(42 blanket module-level, 64 item-level) were pointless: they existed because the lib compiled every module and used none. Removing them all turns the dead-code lint back on for the binary. It flagged 24 genuinely dead items.Deleted (20)
rbac.rs::handle_ldap_commandand fiverbac_impl.rsLDAP functions.enterprise ldapruns throughldap.rs; these were the old implementation.handle_debuginfo_all/node/database. The live command uses the_binaryvariants.parse_database_id, theOptimizationResultstruct + method, two support-package keyring helpers,build_database_payload,WorkflowResult::failure,ConnectionManager::save_config, andWorkflowContext.wait_timeout(set but never read, soinit-clusteralready ignored--wait-timeout).Kept, with a targeted
#[allow(dead_code)]+ comment (5, see #1049)These are not cruft, so they were not cut:
error.rsNoProfileConfigured/MissingCredentials/UnsupportedDeploymentType: intended error types with good messages andsuggestions(), but no code path constructs them (the CLI reports these conditions with generic errors instead). A latent UX improvement to wire up.connection.rsCloudConnectionInfo/EnterpriseConnectionInfocredential fields: populated but never read by the curl builder, soapi ... --curloutput omits its auth headers. That looks like a real bug; kept pending a fix rather than deleted.Result
Net ~670 lines removed. 5 documented allows remain instead of 106 blanket ones, and the dead-code lint now covers the whole crate, so new dead code will be caught going forward.
Validation
cargo fmt --all -- --check,cargo clippy --workspace --all-targets --all-features -- -D warnings,cargo test --workspace --all-features: all pass, zero failures.Follow-ups (flagged, not done here)
api ... --curlto emit auth headers, then the connection-info credential fields become live.