Remove dead helper functions#14
Conversation
run_tracked_command, json_kv, check_disk_space, check_network, safe_write_file, create_mysql_auth_file, get_all_panel_keys, get_site_name and sanitize_for_filename have no callers. get_site_name and sanitize_for_filename were duplicated by the generated-script copies in generators.sh, which is what the backup scripts actually use. Drops the matching sanitize_for_filename unit tests.
get_job_history and get_history_by_jobs have no callers; per-job history reporting was never wired into the CLI.
migration_recommended and get_migration_recommendation have no callers; backupd.sh computes the version comparison inline in its encryption-status and migrate flows.
No callers anywhere; migration generates default-job scripts through generate_job_scripts instead.
The installer never calls get_install_hint. The maintained copy lives in lib/core.sh (used by lib/crypto.sh); the install.sh copy was dead.
Greptile SummaryThis PR removes 369 lines of confirmed-dead helper functions across five files as part of a grouped dead-code cleanup. Every removed symbol was verified to have zero callers in the live codebase, and the matching USAGE.md documentation for the history API was updated in the same changeset, addressing the concern raised in a previous review thread.
Confidence Score: 5/5Safe to merge — every removed function was confirmed to have zero callers, and the USAGE.md documentation was updated in the same changeset. All deletions were cross-referenced against the full codebase: no live caller was found for any removed symbol. The get_install_hint duplicate in install.sh was correctly identified as redundant because the maintained copy in lib/core.sh is what backupd.sh and lib/crypto.sh actually call. The active get_site_name/sanitize_for_tag copies used by the generated backup scripts remain untouched in lib/generators.sh. The only noteworthy side-effect is an unintentional execute-bit removal on four files, which does not affect the documented curl | bash installation path. No files require special attention; the execute-bit change on install.sh is worth a quick git update-index --chmod=+x before merging if direct invocation is expected. Important Files Changed
|
| @@ -112,70 +112,3 @@ get_next_backup_times() { | |||
| local verify_full_next=$(echo "$timer_out" | grep "backupd-verify-full.timer" | awk '{print $1, $2, $3}') | |||
There was a problem hiding this comment.
Documented public API removed without updating USAGE.md
get_job_history and get_history_by_jobs are explicitly advertised in USAGE.md (lines 1222-1235) as a programmatic interface that users are instructed to call by directly sourcing history.sh. Any user script following that documentation will silently get a command not found error after this change. The removal is fine if these functions are truly unused, but USAGE.md needs a matching update to remove (or redirect) those code examples.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Good catch — addressed in 05720dd. get_job_history/get_history_by_jobs had zero callers in the codebase (the CLI's backupd job show <job> computes per-job history with its own inline logic, not these functions), so they were genuinely dead. I've removed the now-stale programmatic examples from USAGE.md's Job-Specific History section and kept the supported sudo backupd job show <job> path. No more dangling source history.sh; get_job_history ... docs that would hit command-not-found.
get_job_history and get_history_by_jobs were removed as dead code; USAGE.md still advertised them as a programmatic interface to source directly, which would now fail with "command not found". Keep the supported CLI path (backupd job show <job>) for per-job history.
Summary
Part of a grouped dead-code cleanup (3 PRs, split by area). This PR removes zero-caller helper functions across several modules.
Changes (5 atomic commits)
lib/core.sh— removerun_tracked_command,json_kv,check_disk_space,check_network,safe_write_file,create_mysql_auth_file,get_all_panel_keys,get_site_name,sanitize_for_filename(the last two were duplicated by the generated-script copies ingenerators.sh, which the backup scripts actually use); drops the matchingsanitize_for_filenameunit testslib/history.sh— removeget_job_history,get_history_by_jobs(per-job history reporting never wired into the CLI)lib/crypto.sh— removemigration_recommended,get_migration_recommendation(backupd.shdoes the version comparison inline)lib/generators.sh— removegenerate_default_job_scripts_from_config(no callers)install.sh— removeget_install_hint(installer never calls it; the maintained copy is inlib/core.sh)369 lines removed.
Scope note
Only functions with zero callers. Two related items were deliberately left out of this batch for a separate review because they are behaviour-adjacent rather than dead: trimming the
lib/logging.shinstrumentation (its internal call graph needs care) and pruning the unusedEXIT_*constants inlib/exitcodes.sh(a small, tested sysexits.h reference table).Testing
./tests/run-tests.sh --syntax— 22 files pass./tests/run-tests.sh --unit— all unit tests pass (162; the 7 removed tests covered the deletedsanitize_for_filename)shellcheck -x -s bash -S warning backupd.sh lib/*.sh— cleanTargets
develop. Hold for green CI + review before merge.