docs: modernize README - #43
Conversation
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HPkbDi3s63kuhdfuNhgS8v
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 89c18fadc9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| This is the same self-test the CI workflow uses. On a host without the `pihole` CLI the run exits | ||
| cleanly instead of failing. |
There was a problem hiding this comment.
Correct the no-CLI selftest promise
This paragraph says a host without the pihole CLI exits cleanly, but pihole_maintenance_pro.sh only returns 0 for a missing CLI when ${CI} is set; otherwise the CLI lookup path exits 127. That makes the advertised first-look selftest fail on a non-Pi-hole laptop/dev host, which is exactly the environment this section is trying to reassure users about.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in 602373b. The selftest section now states that the pihole CLI is required to pass the environment check, that a non-Pi-hole host stops early with exit 127 (unless CI=1, as CI sets), and points to bash -n for a syntax-only check anywhere.
Generated by Claude Code
| Every run writes a timestamped log to `/var/log/pihole_maintenance_pro_<timestamp>.log`. | ||
| Per-step live lines use a unified, timestamped format (`[HH:MM:SS] OK|WARN|ERR …`). Use `--json` | ||
| for a machine-readable result you can pipe into monitoring. |
There was a problem hiding this comment.
Don't promise pipeable JSON output
--json is not a clean JSON stream today: in pihole_maintenance_pro.sh, JSON_OUTPUT is only checked inside the on_exit trap after echo_hdr and the run_step progress/log output have already written human-readable text to stdout. Users following this sentence with something like --json | jq or a monitoring collector will not receive a single parseable JSON document unless they first strip the preceding output.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct — fixed in 602373b. The docs no longer imply a pipeable stream: --json is now described as emitting a JSON block in place of the end-of-run summary, with a note that step/progress lines precede it on stdout, so the trailing JSON must be extracted rather than piping the whole run into jq.
Generated by Claude Code
Address Codex review: the selftest requires the pihole CLI (exit 127 without it unless CI=1), and --json output is preceded by human-readable step lines on stdout, so it is not a clean pipeable document. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HPkbDi3s63kuhdfuNhgS8v
Goal
Redesign
README.mdso a first-time GitHub visitor can, within the first screen, understand whatpihole-maintenance-prois, which problem it solves, who it's for, why it beats a hand-typed maintenance one-liner, and how to test it safely before trusting it. This is a documentation-only change aimed at building trust and improving star conversion.Key structural changes
--backup, per-step opt-outs, exit codes, once-only interrupt cleanup, reversible install).AGENTS.md+make check.skillicons.devimage; kept only the three real badges (Build/License/Buy-Me-a-Coffee).Deliberately NOT changed
pihole_maintenance_pro.shand all other scripts are untouched.README.de.mdleft as-is (English-only scope for this PR — see open risks).Accuracy
All described features, flags, exit codes, paths, and commands were cross-checked against the current code (
pihole_maintenance_pro.shv5.3.2,scripts/install.sh, CIci-sanity.yml,AGENTS.md). Backup behavior is now described precisely: an automatic snapshot to/etc/pihole/backup_<ts>runs before update and gravity, and--backupadds a rotated snapshot under/var/backups/pihole/(keeps 5).Verification (exit codes)
git diff --check→ exit 0 (no whitespace/conflict-marker issues).README.de.md,LICENSE,AGENTS.md,tools/…,scripts/…,.env.exampleall resolve to real files.shellcheck/shfmtare not installed in this environment, somake checkwas not run locally; the change is docs-only and touches no shell scripts, so CI (ci-sanity.yml) covers it.Open risks
README.de.mdnow lags the English redesign; it should be updated in a follow-up for parity.--helpoutput omits--backupand--restart-ftl, though both flags are implemented and work; the README documents them. Minor upstream inconsistency, intentionally left untouched here (docs-only PR).🤖 Generated with Claude Code
https://claude.ai/code/session_01HPkbDi3s63kuhdfuNhgS8v
Generated by Claude Code