Skip to content

refactor: print deploy canister-phase line per state#654

Merged
lwshang merged 2 commits into
mainfrom
spofford/deploy-header-per-state
Jul 16, 2026
Merged

refactor: print deploy canister-phase line per state#654
lwshang merged 2 commits into
mainfrom
spofford/deploy-header-per-state

Conversation

@lwshang

@lwshang lwshang commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #652, from the discussion on the phase-header wording.

Instead of an always-printed header, the canister phase now prints a line that reflects what actually happens:

Invocation State Output
icp deploy all exist All canisters already exist
icp deploy some missing Creating canisters:Created canister …
icp deploy --no-create all exist All canisters already exist
icp deploy --no-create some missing error only (no header)

Why

  • Drops the awkward Ensuring canisters exist: on a normal deploy.
  • Creating canisters: only appears in the branch that actually creates, so it never sits in front of All canisters already exist or the --no-create error.
  • Output follows the state, not the flag: deploy and deploy --no-create print the same thing whenever they do the same thing (all canisters exist), and differ only when the behavior genuinely differs.

No behavior change beyond the log wording; no arg/help changes, so CLI docs are unaffected.

🤖 Generated with Claude Code

Follow-up to #652. Instead of an always-printed phase header, print a line
that reflects what actually happens: "All canisters already exist" when there
is nothing to create, or "Creating canisters:" only in the branch that creates.

This drops the awkward "Ensuring canisters exist:" on a normal deploy, and
avoids "Creating canisters:" ever sitting in front of "All canisters already
exist" or the `--no-create` error. Output now follows the state, not the flag:
`deploy` and `deploy --no-create` print the same thing whenever they do the
same thing (all canisters exist), and differ only when the behavior differs.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 16, 2026 18:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refines deploy-phase logging so messages reflect whether canisters already exist, must be created, or cannot be created due to --no-create.

Changes:

  • Removes the unconditional phase header.
  • Prints Creating canisters: only when creation occurs.
  • Preserves the existing all-canisters-exist message.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread crates/icp-cli/src/commands/deploy.rs
Addresses a Copilot review comment: the existing --no-create tests only
checked success/failure, so a regression that printed "Creating canisters:"
before the --no-create error (or dropped "All canisters already exist") would
pass silently.

- missing + --no-create: error prints with no "Creating canisters:" header.
- creating deploy: prints "Creating canisters:".
- all-exist + --no-create: prints "All canisters already exist", no header.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@lwshang
lwshang marked this pull request as ready for review July 16, 2026 18:27
@lwshang
lwshang requested a review from a team as a code owner July 16, 2026 18:27
@lwshang
lwshang enabled auto-merge (squash) July 16, 2026 18:27
@lwshang
lwshang merged commit ff58b1d into main Jul 16, 2026
92 checks passed
@lwshang
lwshang deleted the spofford/deploy-header-per-state branch July 16, 2026 18:31
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.

3 participants