feat: Add --no-create flag to icp deploy#652
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an intended --no-create safeguard to icp deploy.
Changes:
- Adds missing-canister validation.
- Documents the flag in the changelog.
- Regenerates the CLI reference, but leaves it truncated.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
crates/icp-cli/src/commands/deploy.rs |
Adds deployment guard logic. |
CHANGELOG.md |
Announces the new flag. |
docs/reference/cli.md |
Updates generated CLI documentation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
adamspofford-dfinity
force-pushed
the
spofford/flag-no-create
branch
from
July 16, 2026 00:03
b32b26e to
18c4505
Compare
Member
|
This looks solid overall — clean implementation, good test coverage (both the missing and already-exists paths), and CHANGELOG + CLI docs are updated. 👍 Leaving final approval to Linwei/Raymond since this introduces a new flag. A few optional, non-blocking nits:
None of these block merge. |
The unconditional "Creating canisters:" header over-claimed the phase: it printed even when nothing would be created, which read oddly ahead of both "All canisters already exist" and the new --no-create error. Rewording to "Ensuring canisters exist:" makes every follow-up line read naturally, and lets the "already exist" message print unconditionally again (dropping the --no-create special case). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
--subnet and --cycles only affect canister creation, so combining them with --no-create silently ignored them. Reject the combination at parse time so the user gets immediate feedback. --proxy is intentionally excluded since it is used well beyond creation (install, settings, sync). The defaulted --cycles only conflicts when explicitly passed, not via its default value. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Locks in the parse-time conflict added for --no-create. The explicit --cycles case also guards against a future regression where the flag's default value could start spuriously triggering the conflict. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
lwshang
approved these changes
Jul 16, 2026
lwshang
enabled auto-merge (squash)
July 16, 2026 17:22
lwshang
added a commit
that referenced
this pull request
Jul 16, 2026
* refactor: drop universal deploy header, print per state 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> * test: assert deploy canister-phase messages per state 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> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.