Clarify install.sh binary path and pre-install config customization in README#32
Merged
Merged
Conversation
The README's placeholder path/to/pimonitor-arm64 suggested an architecture-suffixed filename, but release tarballs ship the binary plainly named "pimonitor". It also wasn't obvious that the argument must point to the binary file itself rather than a directory containing it, which led to a "binary not found" error when a directory was passed instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019tj5emEtos6unD4AdufXfJ
install.sh's systemctl enable --now returns immediately for Type=simple units, so a bind failure (e.g. listen_addr port already in use) never surfaces as an install error - the service just crash-loops silently. Document how to stage a customized config.yaml before installing, and how to verify the service actually stayed up afterwards. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019tj5emEtos6unD4AdufXfJ
Add a copy-pasteable snippet using wget (plus curl to resolve the latest release tag via the GitHub API) so users don't need to browse the Releases page and manually pick the right asset filename, which embeds both the version and architecture (pimonitor_<version>_linux_<arch>.tar.gz). Also reorder the installation section into numbered steps (download, optional config customization, run installer, verify) so the config customization step is clearly presented before install.sh runs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019tj5emEtos6unD4AdufXfJ
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
2 participants
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
packaging/install.shmust be the path to the binary file itself (not a directory, and not the tarball), and correct the misleadingpath/to/pimonitor-arm64placeholder — release tarballs ship the binary plainly namedpimonitor, with no architecture suffix.install.shnever overwrites an existing/etc/pimonitor/config.yaml, so users who want non-default settings (e.g. a differentlisten_addr) from the start should stage the config file themselves before running the script.systemctl enable --nowreturns immediately forType=simpleunits, so a startup failure (most commonly alisten_addrport already in use) never surfaces as an install error — the service just crash-loops silently (Restart=on-failure, retried every 5s). Document how to spot this (systemctl statusshowingactivating (auto-restart)) and diagnose it (journalctl -u pimonitor -n 50).packaging/install.shitself.These came out of a real installation issue: a user pointed
install.shat a directory instead of a file (misled by thepath/to/pimonitor-arm64placeholder), and separately had port8080already in use on their Pi with no error reported during installation.Related Issue
None — user-reported installation confusion, docs-only fix.
Checklist
go test ./...passes locally)go vet ./...andgolangci-lint runare cleandocs/API.md),configuration (
README.md,packaging/pimonitor.example.yaml), orinstallation/packaging (
packaging/install.sh, systemd units)/api/v1/...response shapes, or a new APIversion was introduced instead
This is a documentation-only change (plus a comment in
install.sh); no Go source was touched, so there is nothing to test/vet/lint.