Skip to content

Makefile: fixed setup_test_to_use_xxx to only clean up the cli containers and not all containers#837

Merged
ToreMerkely merged 2 commits intomainfrom
fix-setup-test-to-use-local-image-scope
Apr 29, 2026
Merged

Makefile: fixed setup_test_to_use_xxx to only clean up the cli containers and not all containers#837
ToreMerkely merged 2 commits intomainfrom
fix-setup-test-to-use-local-image-scope

Conversation

@ToreMerkely
Copy link
Copy Markdown
Contributor

  • fix(make): scope setup_test_to_use_local_image cleanup to the compose stack
  • fix(make): scope setup_test_to_use_staging_server_image cleanup to the compose stack

ToreMerkely and others added 2 commits April 29, 2026 11:41
… stack

The previous target ran `docker ps -aq | xargs -r docker rm -fv`,
which force-removes EVERY container on the host — not just the kosli
test stack defined in docker-compose.yml. That's a footgun for anyone
running unrelated containers (other projects, dev DBs, sidecar tools)
on the same machine.

`docker compose down -v` only touches the project's own containers
plus its named volumes — exactly what's needed when switching to a
locally-built server image. Same approach already used by
bin/reset-or-start-server.sh.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…e compose stack

Same broad-cleanup footgun as setup_test_to_use_local_image fixed in
the previous commit — `docker ps -aq | xargs -r docker rm -fv` force-
removes EVERY container on the host. Replaced with `docker compose
down -v`, which only touches the project's own compose stack.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@ToreMerkely ToreMerkely enabled auto-merge (squash) April 29, 2026 09:45
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 29, 2026

Claude finished @ToreMerkely's task in 1m 38s —— View job


PR Review: Scope Makefile cleanup to compose stack

  • Run static analysis (build, vet, mod tidy) — all clean
  • Review the Makefile changes
  • Post review feedback

Static Analysis

All checks pass — go build, go vet, and go mod tidy report no issues.

Review

The change is correct and well-scoped. The old commands (docker ps -aq | xargs -r docker rm -fv) were a sledgehammer — they force-removed every container on the host, regardless of whether it belonged to this project. Replacing them with docker compose down -v limits cleanup to the containers, networks, and volumes defined in docker-compose.yml, which is exactly the right scope for these targets.

This also aligns with how bin/reset-or-start-server.sh already manages the stack (it uses docker compose down and docker compose up -d), so the Makefile is now consistent with existing patterns.

No issues found. The change is minimal, safe, and solves a real problem (e.g., accidentally killing unrelated containers during local development). LGTM.

@ToreMerkely ToreMerkely merged commit beda1fd into main Apr 29, 2026
19 of 21 checks passed
@ToreMerkely ToreMerkely deleted the fix-setup-test-to-use-local-image-scope branch April 29, 2026 09:54
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