fix: rewrite only the version line when syncing OpenAPI specs - #28
ChrisSchinnerl wants to merge 1 commit into
Conversation
yq -i reserializes the entire document. On the Sia specs that collapses every hand-wrapped block scalar onto one line and strips the blank lines between sections, so a one-line version bump arrives as a whole-file reformat — 40 changed lines for s3d, which is 153 lines long. Locate info.version by line and rewrite just that line, then read the file back with yq to confirm it still parses and reports the new version. If the key can't be found, warn and skip the file rather than write something unintended. Matching is anchored to the indent of info's own keys, so a `version:` nested deeper — inside a description block scalar, for instance — is never mistaken for the real one. Verified against all eight specs in the org (renterd, hostd, walletd, explored, vaultd, s3d, indexd app and admin): exactly one line changes in each, and the parsed documents are otherwise byte-identical. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
Two unresolved findings may skip valid files or produce invalid numeric version values.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Updates the OpenAPI sync workflow to rewrite only the info.version line while preserving document formatting.
Changes:
- Replaces whole-document writes with line-based updates.
- Validates the updated YAML and version.
- Skips files where
info.versionis not found.
File summaries
| File | Summary | Findings |
|---|---|---|
.github/workflows/sync-openapi-version.yml |
Implements formatting-preserving version synchronization. | Critical finding (3 votes): quote the version scalar to preserve string typing. Moderate finding (2 votes): ignore comments between info: and its child keys. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if (indent === null) indent = m[1]; | ||
| if (m[1] !== indent) continue; | ||
| if (new RegExp(`^${indent}version:\\s*(.*)$`).test(line)) { | ||
| lines[i] = `${indent}version: ${version}`; |
| if (!line.trim()) continue; | ||
| const m = line.match(/^(\s+)\S/); | ||
| if (!m) break; |
|
Closing — we're accepting the one-time reformat instead. The churn is one-time per repo: after the first sync PR lands, each spec is in yq's canonical style and every subsequent bump is a clean two-line diff. The reformat is semantically lossless — parsed documents are byte-identical apart from the version — so the only real cost is that the specs stop being hand-wrapped. Not worth an extra tag and a second round of pin bumps across seven repos. The behaviour fix in #27 is the part that mattered. |
Follow-up to #27. That fix made the workflow actually run; this one makes its output reviewable.
Problem
yq -ireserializes the entire document. On the Sia specs that collapses every hand-wrapped>block scalar onto a single line and strips the blank lines between sections. The first real run — s3d 35227139924 — correctly bumped0.1.1 -> 0.1.4, but the PR it opened (SiaFoundation/s3d#269, since closed) touched 40 lines of a 153-line file. Values were unchanged, but nobody wants to review that on every release, and it would land in all seven consuming repos.Fix
Locate
info.versionby line and rewrite only that line.yqis still used to read the current version, and now also to read the file back afterwards to confirm it parses and reports the new value.Matching is anchored to the indent of
info's own keys, so aversion:nested deeper — inside adescriptionblock scalar, for example — can't be mistaken for the real one. If the key isn't found, the file is skipped with a warning rather than written speculatively.Verification
Extracted the helper from the YAML as it will actually be parsed and ran it against all eight specs in the org — renterd, hostd, walletd, explored, vaultd, s3d, and both indexd specs:
After merge
Needs a new tag —
v0.1.0currently points at #27's commit, and consumers pin by SHA with a# v0.1.0comment. s3d is already onv0.1.0and will run this workflow on its next release, so it would hit the reformat until its pin moves.🤖 Generated with Claude Code