Skip to content

fix(display): wire --dry-run flag into progress writers - #14053

Open
htoyoda18 wants to merge 1 commit into
docker:mainfrom
htoyoda18:fix/dry-run-progress-display
Open

fix(display): wire --dry-run flag into progress writers#14053
htoyoda18 wants to merge 1 commit into
docker:mainfrom
htoyoda18:fix/dry-run-progress-display

Conversation

@htoyoda18

@htoyoda18 htoyoda18 commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

What I did

The tty, plain, and json EventProcessor constructors (display.Full, display.Plain, display.JSON) never accepted a dry-run flag, so the "DRY-RUN MODE" prefix was never rendered in any progress output even when --dry-run was passed. Added a dryRun bool parameter to all three constructors and threaded the already-parsed --dry-run flag value through selectEventProcessor in cmd/compose/compose.go. Also removed the related FIXME(ndeloof) comment in cmd/display/tty.go now that it's wired up.

Added unit tests in cmd/display (plain_test.go, json_test.go, tty_test.go) verifying each constructor wires dryRun through to its writer.

Related issue
N/A

(not mandatory) A picture of a cute animal, if possible in relation to what you did
🐈

@htoyoda18
htoyoda18 requested review from a team as code owners August 16, 2026 12:53
@htoyoda18
htoyoda18 requested review from glours and ndeloof August 16, 2026 12:53
The tty, plain, and json EventProcessor constructors accepted no
dry-run flag, so the DRY-RUN prefix was never rendered even when
--dry-run was passed.

Signed-off-by: hiroto.toyoda <hiroto.toyoda@dena.com>
@htoyoda18
htoyoda18 force-pushed the fix/dry-run-progress-display branch from 1454b74 to 03831bc Compare August 23, 2026 13:10
@htoyoda18

Copy link
Copy Markdown
Contributor Author

Hi @ndeloof — this fixes the FIXME you left in cmd/display/tty.go (dryRun field was never wired up across the tty/plain/json progress writers, so --dry-run never showed the DRY-RUN prefix in progress output). I've since rebased on top of your PersistentPreRunE refactor and the display.Mode resolution changes, so it should apply cleanly. Would appreciate a review when you have a chance — thanks!

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.

1 participant