Skip to content

Make ./up.sh and ./down.sh run as documented - #295

Merged
dmccoystephenson merged 1 commit into
mainfrom
feature/compose-v2-helper-scripts
Aug 14, 2026
Merged

Make ./up.sh and ./down.sh run as documented#295
dmccoystephenson merged 1 commit into
mainfrom
feature/compose-v2-helper-scripts

Conversation

@dmccoystephenson

Copy link
Copy Markdown
Member

Summary

No behaviour of the site itself is touched: the diff is three shell scripts and the changelog, with no TypeScript, no component and no configuration change.

Test plan

  • Script syntax and content reviewed by reading the full diff.
  • Recorded modes verified as 100755 in the committed tree (git show --summary reports the three mode changes).
  • The Compose v2 spelling was checked against compose.yml and against every document that names a Compose command, so nothing else needs updating alongside it.
  • README.md (lines 108, 125, 131) and CONFIG.md (lines 48, 64) were re-read: both already document ./up.sh and ./down.sh, and both become accurate with this change rather than needing an edit.
  • Lint, unit tests and build are left to CI (build.yml runs npm run lint, npm test and npm run build); no Node toolchain was available in the environment where this was prepared.
  • A real local run of ./up.sh and ./down.sh against a Docker installation is still wanted before merge, as issue up.sh/down.sh call the retired docker-compose v1 binary and have a commented-out shebang #292 asks — Compose is exercised by neither CI nor the environment this was prepared in.

Closes #292
Closes #294

This PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

README.md gives ./up.sh as the way to start the local stack, but none of
the three helper scripts were tracked as executable, so the command
answered "Permission denied" on a fresh clone. Their first line was
"# /bin/bash" -- a comment rather than a shebang -- and the two Compose
scripts called docker-compose, the v1 binary retired in June 2023 that a
current Docker installation may not provide at all.

Closes #292
Closes #294

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@dmccoystephenson

Copy link
Copy Markdown
Member Author

Self-review

The diff was read in full and each claim in the pull-request description was re-checked against source. No blocking defect was found. Three notes are recorded below, two of which describe pre-existing conditions deliberately left out of scope, and one of which is the verification gap the description already flags.

Correctness of the change itself. The v2 spelling is right in both scripts: docker compose up --remove-orphans --build -d and docker compose down --remove-orphans take the same flags under v2 as under v1, so no invocation semantics change beyond the binary. The mode change is recorded in the committed tree rather than only on a working copy — git show --summary on the commit reports mode change 100644 => 100755 for all three files — which is what makes the fix survive a clone.

Documentation consistency. README.md line 108 promises that ./up.sh rebuilds; --build is retained, so that stays true. CONFIG.md line 48 says ./up.sh aborts without JWT_SECRET, and line 64 pairs ./up.sh with docker compose up --build; both remain accurate and neither needs an edit. The CHANGELOG.md entry is placed under [Unreleased] after Changed, which is the Keep a Changelog ordering.

Notes

build_and_run.sh:8 — finding, out of scope. The script has no set -e, so a failing npm run build is followed by npm run start regardless, and the old build having just been deleted by rm -rf .next at line 5, the result is a start against no build at all. This predates the change and was not part of #292 or #294, so it is left alone here rather than widened into a behaviour change nobody has reviewed. It is worth a follow-up issue.

build_and_run.sh:1 — observation. This script is referenced by no document in the repository: README.md, CONFIG.md, CONTRIBUTING.md and USER_GUIDE.md name only ./up.sh and ./down.sh. Its shebang is corrected here because #292 asks for it, but whether an undocumented duplicate of npm run build && npm run start should be kept at all is a separate question.

up.sh:5 — verification gap, already disclosed. A Compose invocation cannot be exercised by this repository's CI, which runs lint, tests and build for the website and ./mvnw verify for the API but starts no Docker stack. Nor was Docker, npm or node available where this change was prepared. The substitution of docker compose for docker-compose is therefore reviewed rather than run, and the unchecked box in the test plan asks for a real local ./up.sh before merge, as #292 itself requests.

This comment was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

@dmccoystephenson

Copy link
Copy Markdown
Member Author

Held for a local Compose run, not merged

Both CI jobs pass — Build Website (Node.js) and Build & Test API (Java) — the self-review above found no blocking defect, and the documentation pass over CHANGELOG.md, CONFIG.md, README.md and USER_GUIDE.md found nothing to correct. This pull request is otherwise ready.

It is being left open rather than merged because #292 asks for something this run cannot supply:

Verification of a Compose invocation needs Docker, which CI does not exercise and which was not available where this was found — so a real local run of ./up.sh should confirm the change rather than review alone.

That remains true here. build.yml starts no Docker stack, and no Docker installation was reachable from the environment this change was prepared in, so the substitution of docker compose for docker-compose has been reviewed but never run. Merging on a green CI result would be reading a pass from checks that do not exercise Compose as though it covered the one thing the issue asked to see exercised.

What is wanted before merge is a single local check on a machine with Docker: ./up.sh should bring the stack up with JWT_SECRET set, and ./down.sh should take it down again. The executable-bit half of the change (#294) needs no such confirmation — the recorded modes are visible in the diff as 100644 => 100755.

This comment was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

@dmccoystephenson
dmccoystephenson merged commit 3be06d4 into main Aug 14, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant