Repository navigation
Replace the Preview name positional argument with --preview-name - #519
Conversation
|
LGTM! |
commit: |
|
| mean | stddev | min | max | |
|---|---|---|---|---|
| PR | 283.3 ms | 2.9 ms | 279.4 ms | 290.9 ms |
| base | 285.4 ms | 3.5 ms | 280.0 ms | 294.4 ms |
Δ (PR vs base): ↓ -2.1 ms (-0.7%)
Measured with hyperfine on ubuntu-latest (3 warmup runs, 20 timed runs). CI numbers carry ±a few ms of runner jitter; treat small deltas as noise.
There was a problem hiding this comment.
Superseded by the review of
ffe3d5ff.
This review, of a9e52a83
✅ Approved — The positional argument is replaced with the --preview-name option everywhere: the command code, the metadata, the generated JSON, the error text, the tests and the changeset. No issues found.
The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.
✅ All checks passed
🚀 Release: ✅ LGTM
Risk: Low. The minor changeset matches the repository's rule for pre-1.0 behaviour changes and clearly describes the breaking change.
🔒 Security: ✅ LGTM
No security-relevant changes.
📖 AGENTS.md: ✅ LGTM
The packages/cli/AGENTS.md update matches the new flag.
📋 Code Quality: ✅ LGTM
The builder, command string, metadata sidecar and generated metadata are in sync, and the tests cover the new flag.
⚡ Performance: ✅ LGTM
No performance impact.
📝 Documentation: ✅ LGTM
The error message and usage strings now show the flag syntax.
Reviewed commit: a9e52a83
edmundhung
left a comment
There was a problem hiding this comment.
This will need follow-ups in two places that still use the positional form:
- the docs (/cf/projects/ and /cf/ci/)
@vinext/cloudflarewhich runscf previews deploy <name>forvinext-cloudflare previews deploy <name>.
a9e52a8 to
ffe3d5f
Compare
There was a problem hiding this comment.
Superseded by the review of
00bd1fe4.
This review, of ffe3d5ff
✅ Approved — The positional argument is replaced with the --preview-name option everywhere: the command code, the metadata, the generated JSON, the error text, the tests and the changeset. No issues found.
The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.
✅ All checks passed
🚀 Release: ✅ LGTM
Risk: Low. The minor changeset matches the repository's rule for pre-1.0 behaviour changes and clearly describes the breaking change.
🔒 Security: ✅ LGTM
No security-relevant changes.
📖 AGENTS.md: ✅ LGTM
The packages/cli/AGENTS.md update matches the new flag.
📋 Code Quality: ✅ LGTM
The builder, command string, metadata sidecar and generated metadata are in sync, and the tests cover the new flag.
⚡ Performance: ✅ LGTM
No performance impact.
📝 Documentation: ✅ LGTM
The error message and usage strings now show the flag syntax.
Reviewed commit: ffe3d5ff — same changes as a9e52a83, so this is that commit's review (no new review was run; line numbers are as of a9e52a83)
|
LGTM! |
ffe3d5f to
00bd1fe
Compare
Auto-merge was disabled for another pull request in the stack
|
LGTM |
There was a problem hiding this comment.
✅ Approved — The positional argument is replaced with the --preview-name option everywhere: the command code, the metadata, the generated JSON, the error text, the tests and the changeset. No issues found.
The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.
✅ All checks passed
🚀 Release: ✅ LGTM
Risk: Low. The minor changeset matches the repository's rule for pre-1.0 behaviour changes and clearly describes the breaking change.
🔒 Security: ✅ LGTM
No security-relevant changes.
📖 AGENTS.md: ✅ LGTM
The packages/cli/AGENTS.md update matches the new flag.
📋 Code Quality: ✅ LGTM
The builder, command string, metadata sidecar and generated metadata are in sync, and the tests cover the new flag.
⚡ Performance: ✅ LGTM
No performance impact.
📝 Documentation: ✅ LGTM
The error message and usage strings now show the flag syntax.
Reviewed commit: 00bd1fe4 — same changes as a9e52a83, so this is that commit's review (no new review was run; line numbers are as of a9e52a83)
cf previews deploy replaced its positional Preview name with --preview-name (cloudflare/cf#519). Mirror that in vinext-cloudflare previews deploy and pass the name to cf with the flag.
Replace the Preview name positional argument with
--preview-nameThis is a breaking change during the 1.0 beta: use
cf previews deploy --preview-name featureinstead ofcf previews deploy feature. Omitting the flag still defaults to the current CI or Git branch; when no branch is available, the error now shows the flag syntax.