Skip to content

Fix containerCommit() to not pause container if $pause is false - #103

Merged
clue merged 1 commit into
clue:1.xfrom
clue-labs:commit-pause
Oct 18, 2026
Merged

clue merged 1 commit into
clue:1.xfrom
clue-labs:commit-pause

Conversation

@clue

@clue clue commented Oct 18, 2026

Copy link
Copy Markdown
Owner

This changeset fixes containerCommit() to not pause the container if $pause is false. It previously sent an empty pause= parameter, which Docker 25 (2024) and older treat like the default and pause the container anyway, so the option had no effect there. It now sends pause=0, which works on all Docker versions. Docker 26 (2024) and newer already treated the empty value as false. This has been broken since containerCommit() was added in #74.

Builds on top of #74

@clue clue added the bug Something isn't working label Oct 18, 2026
@clue clue added this to the v1.7.0 milestone Oct 18, 2026
@clue
clue requested a balanced review from Copilot October 18, 2026 01:11

Copilot AI 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.

🟢 Approval recommended

The focused fix addresses the serialization issue and includes regression coverage, with no unresolved findings.

No open issues 🎉

What changed in this PR

Fixes containerCommit() so $pause = false prevents pausing on older Docker versions while preserving default behavior.

Changes:

  • Serializes pause explicitly as 1 or 0.
  • Adds regression coverage verifying pause=0 when pausing is disabled.
File Description
tests/​ClientTest.php Adds a regression test for committing without pausing.
src/​Client.php Encodes the pause flag as an explicit numeric value.

Review effort: Balanced

@clue
clue merged commit c319415 into clue:1.x Oct 18, 2026
15 checks passed
@clue
clue deleted the commit-pause branch October 18, 2026 01:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants