Repository navigation
chore: update OpenAPI to 305b054ec4ae - #528
Conversation
commit: |
There was a problem hiding this comment.
Superseded by the review of
ce97954e.
This review, of 11de7a96
✅ Approved — This is a mechanical regeneration from the pinned OpenAPI revision, and the generated files match their metadata. 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
📋 Code Quality: ✅ LGTM
The regenerated commands match the updated metadata in commands.json, and the version pin in generate.ts is updated.
📝 Documentation: ✅ LGTM
The changeset and the updated help text match the new API descriptions.
Reviewed commit: 11de7a96
|
LGTM |
|
| mean | stddev | min | max | |
|---|---|---|---|---|
| PR | 313.8 ms | 5.5 ms | 302.8 ms | 321.5 ms |
| base | 326.1 ms | 5.9 ms | 316.0 ms | 335.8 ms |
Δ (PR vs base): ↓ -12.3 ms (-3.8%)
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.
11de7a9 to
ce97954
Compare
|
LGTM |
There was a problem hiding this comment.
Superseded by the review of
1f3565ed.
This review, of ce97954e
💬 Approved with comments — This is a mechanical regeneration, and the generated files match their metadata. The one note is an INFO: the changeset doesn't mention the --limit/--offset flags removed from ssl origin-ca-certificates list.
Findings: 1 info (1 is an inline comment on the diff)
The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.
🚀 Release: 💬 1 info
The changeset doesn't mention the removed --limit/--offset flags
Recommendation: 💬 Optional Nits
- [INFO] The changeset doesn't mention that
--limitand--offsetwere removed fromcf ssl origin-ca-certificates list. (.changeset/update-openapi-93efa9a39e4c.md:5)
View full review
The minor bump is the right level for a pre-1.0 behaviour change. However, .changeset/README.md says breaking changes during the beta must be called out explicitly. This changeset only says the generated command surface was updated, while cf ssl origin-ca-certificates list no longer accepts --limit or --offset.
✅ All other checks passed
🔒 Security: ✅ LGTM
No security concerns in the regenerated command surface.
📖 AGENTS.md: ✅ LGTM
No agent instruction files changed.
📋 Code Quality: ✅ LGTM
The regenerated commands match the updated commands.json and schemas.json. containers usage, the create body with its new zone-id field, the removed list flags and the new include values are consistent across the metadata, the operation files and the command files.
⚡ Performance: ✅ LGTM
Only generated command definitions and metadata change; there is no new hot-path work.
📝 Documentation: ✅ LGTM
The updated help text matches the new API descriptions.
Reviewed commit: ce97954e
ce97954 to
1f3565e
Compare
|
LGTM |
There was a problem hiding this comment.
Superseded by the review of
113c3a3b.
This review, of 1f3565ed
💬 Approved with comments — The generated files are consistent with each other. The only finding is an INFO: the changeset doesn't mention the flags removed from ssl origin-ca-certificates list.
Findings: 1 info
The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.
🚀 Release: 💬 1 info
The changeset doesn't mention the flags removed from ssl origin-ca-certificates list.
Recommendation: 💬 Optional Nits
- [INFO] The changeset doesn't mention that
--limitand--offsetwere removed fromcf ssl origin-ca-certificates list. (.changeset/update-openapi-2b4a8d44b50c.md:5)
View full review
The minor bump is the right level for a pre-1.0 behaviour change. However, .changeset/README.md:33 says breaking changes during the 1.0 beta must be called out explicitly, and cf ssl origin-ca-certificates list no longer accepts --limit or --offset.
✅ All other checks passed
🔒 Security: ✅ LGTM
There are no security concerns in the regenerated command surface.
📖 AGENTS.md: ✅ LGTM
No agent instruction files changed.
📋 Code Quality: ✅ LGTM
containers usage, the new zone-id body field on ssl origin-ca-certificates create, the removed list flags, the include enum on workers versions get and the description-only changes all match between the command files, the operation files and the metadata.
⚡ Performance: ✅ LGTM
Only generated command definitions and metadata change; nothing new runs on a hot path.
📝 Documentation: ✅ LGTM
The updated help text matches the new API descriptions. The changeset wording is covered in the Release section.
Reviewed commit: 1f3565ed
1f3565e to
113c3a3
Compare
|
No new concerns since the last review |
There was a problem hiding this comment.
Superseded by the review of
2ebc87b0.
This review, of 113c3a3b
💬 Approved with comments — The regenerated files are consistent with each other. The only finding is an INFO: the changeset doesn't call out the flags removed from ssl origin-ca-certificates list.
Findings: 1 info
The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.
🚀 Release: 💬 1 info
The changeset doesn't call out the removed --limit and --offset flags.
Recommendation: 💬 Optional Nits
- [INFO] The changeset doesn't mention that
--limitand--offsetwere removed fromcf ssl origin-ca-certificates list. (.changeset/update-openapi-cbac6d46bdae.md:5)
View full review
Risk: Low
The minor bump is the right level for a pre-1.0 behaviour change. However, .changeset/README.md:33 says breaking changes during the 1.0 beta must be called out explicitly, and cf ssl origin-ca-certificates list no longer accepts --limit or --offset.
✅ All other checks passed
🔒 Security: ✅ LGTM
The regenerated command surface raises no security concerns.
📖 AGENTS.md: ✅ LGTM
No agent instruction files changed.
📋 Code Quality: ✅ LGTM
These all match across the command files, the operation files and the containers/index.ts registration:
- the new
containers usagecommand - the
zone-idbody field onssl origin-ca-certificates create - the removed
listflags - the
includeenum onworkers versions get - the description-only changes
The new containers usage command doesn't collide with any hand-written command.
⚡ Performance: ✅ LGTM
Only generated command definitions and metadata change.
📝 Documentation: ✅ LGTM
The updated help text matches the new API descriptions. The changeset wording is covered in the Release section.
Reviewed commit: 113c3a3b
113c3a3 to
2ebc87b
Compare
There was a problem hiding this comment.
Superseded by the review of
a1ecddba.
This review, of 2ebc87b0
💬 Approved with comments — The regenerated files are consistent with each other and with the bumped OpenAPI pin. The only finding is an INFO: the new changeset still doesn't call out the flags removed from ssl origin-ca-certificates list.
Findings: 1 info
The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.
🚀 Release: 💬 1 info
The changeset still omits the removed --limit and --offset flags
Recommendation: 💬 Optional Nits
- [INFO] The changeset doesn't mention that
--limitand--offsetwere removed fromcf ssl origin-ca-certificates list. (.changeset/update-openapi-520da64158d8.md:5)
View full review
Risk: Low
The minor bump is the right level. The new changeset, like the one it replaces, doesn't mention that cf ssl origin-ca-certificates list no longer accepts --limit or --offset. .changeset/README.md says breaking changes during the 1.0 beta must be called out explicitly.
✅ All other checks passed
🔒 Security: ✅ LGTM
The regenerated command surface raises no security concerns.
📖 AGENTS.md: ✅ LGTM
No agent instruction files changed.
📋 Code Quality: ✅ LGTM
packages/cli/generate.ts now pins the OpenAPI revision named in the description. These changes match across the command files, the operation files and the metadata JSON:
- the new
containers usagecommand, registered incontainers/index.ts - the
zone-idbody field onssl origin-ca-certificates create - the removed
listflags - the
includeenum onworkers versions get - the description-only changes
⚡ Performance: ✅ LGTM
Only generated command definitions and metadata change.
📝 Documentation: ✅ LGTM
The updated help text matches the new API descriptions. The Release section covers the changeset wording.
Reviewed commit: 2ebc87b0
|
LGTM |
2ebc87b to
a1ecddb
Compare
|
LGTM |
There was a problem hiding this comment.
💬 Approved with comments — The regenerated files match each other and the new OpenAPI pin. The only note is an INFO: the changeset doesn't call out the removed --limit and --offset flags.
Findings: 1 info
The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.
🚀 Release: 💬 1 info
The changeset still doesn't mention the removed list flags.
Recommendation: 💬 Optional Nits
- [INFO] The changeset doesn't mention that
--limitand--offsetwere removed fromcf ssl origin-ca-certificates list. (.changeset/update-openapi-305b054ec4ae.md:5)
View full review
Risk: Low
The minor bump is the right level. The changeset doesn't mention that cf ssl origin-ca-certificates list no longer accepts --limit or --offset. With strict yargs, scripts that pass either flag will now fail. .changeset/README.md says breaking changes during the 1.0 beta must be called out explicitly.
✅ All other checks passed
🔒 Security: ✅ LGTM
The generator still downloads the OpenAPI asset by its pinned commit hash. containers usage is a read-only, authenticated GET. The zone-id field goes through the same body handling as the other body fields.
📖 AGENTS.md: ✅ LGTM
No agent instruction files changed.
📋 Code Quality: ✅ LGTM
The regenerated changes match across the command files, the operation files and both metadata JSON files:
containers usage: the new command is registered incontainers/index.ts.ssl origin-ca-certificates create: the newzone-idfield is wired into both the dry-run body and the real body.ssl origin-ca-certificates list: the--limitand--offsetflags are removed.abuse-reports mitigations review: it gains a 409ConflictErrormapping.workers versions get: theincludechoices are updated.
⚡ Performance: ✅ LGTM
Only generated command definitions and metadata change. The new command makes a single GET request.
📝 Documentation: ✅ LGTM
The updated help text matches the new API descriptions. The Release section covers the changeset wording.
Reviewed commit: a1ecddba
Updates cf to
openapi@305b054ec4ae4f706ed1270d0d18a45a8c41dd5c.305b054ec4ae4f706ed1270d0d18a45a8c41dd5cThis PR is maintained automatically by the Update OpenAPI workflow.