Skip to content

docs: modernize README - #43

Open
TimInTech wants to merge 2 commits into
mainfrom
claude/readme-modern-redesign-hqr0d0
Open

docs: modernize README#43
TimInTech wants to merge 2 commits into
mainfrom
claude/readme-modern-redesign-hqr0d0

Conversation

@TimInTech

Copy link
Copy Markdown
Owner

Goal

Redesign README.md so a first-time GitHub visitor can, within the first screen, understand what pihole-maintenance-pro is, 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

  • Hero + honest claim and a concise What & why (2–4 sentences) plus a Who it's for section, all above the fold.
  • New "Why this over a plain maintenance one-liner" comparison table (test-before-trust, backups, health checks, cron reliability, interrupt handling).
  • Quick Start reordered to safety-first: non-destructive selftest → installer → real run.
  • Dedicated Safety principles section (selftest, pre-change backup + optional rotated --backup, per-step opt-outs, exit codes, once-only interrupt cleanup, reversible install).
  • Flags and Exit codes promoted to reference tables; new Output/logs/JSON section (real sample dashboard kept verbatim).
  • Trimmed/re-flowed Installation, Update/Uninstall, Scheduling, Pi-hole v6 API, Troubleshooting, and added a short Contributing section pointing to AGENTS.md + make check.
  • Removed the decorative external skillicons.dev image; kept only the three real badges (Build/License/Buy-Me-a-Coffee).

Deliberately NOT changed

  • No script logic changespihole_maintenance_pro.sh and all other scripts are untouched.
  • No CI, tooling, tag, release, or issue changes.
  • README.de.md left 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.sh v5.3.2, scripts/install.sh, CI ci-sanity.yml, AGENTS.md). Backup behavior is now described precisely: an automatic snapshot to /etc/pihole/backup_<ts> runs before update and gravity, and --backup adds a rotated snapshot under /var/backups/pihole/ (keeps 5).

Verification (exit codes)

  • git diff --checkexit 0 (no whitespace/conflict-marker issues).
  • Internal links checked manually — README.de.md, LICENSE, AGENTS.md, tools/…, scripts/…, .env.example all resolve to real files.
  • Every command/flag in the README verified to exist in the repo.
  • shellcheck / shfmt are not installed in this environment, so make check was not run locally; the change is docs-only and touches no shell scripts, so CI (ci-sanity.yml) covers it.

Open risks

  • README.de.md now lags the English redesign; it should be updated in a follow-up for parity.
  • The script's --help output omits --backup and --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

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HPkbDi3s63kuhdfuNhgS8v

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread README.md Outdated
Comment on lines +147 to +148
This is the same self-test the CI workflow uses. On a host without the `pihole` CLI the run exits
cleanly instead of failing.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread README.md Outdated
Comment on lines +181 to +183
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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants