fix: properly version json schemas#171
Conversation
WalkthroughThe generated ChangesSpecs schema metadata
Estimated code review effort: 1 (Trivial) | ~2 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🛑 Changes requested — automated reviewThe patch introduces unversioned JSON schemas that no longer describe the JSON accepted by the CLI and specs runner. This breaks the schema contract even though the Go code itself is unchanged. |
NumaryBot
left a comment
There was a problem hiding this comment.
NumaryBot posted 2 new inline findings.
Summary: #171 (comment)
| @@ -21,35 +21,18 @@ | |||
| }, | |||
| "definitions": { | |||
| "Balances": { | |||
There was a problem hiding this comment.
🟠 [major] Keep the inputs schema aligned with CLI JSON
When users follow inputs.schema.json, this now validates balances as an account-to-asset object map, but internal/cmd/run.go still unmarshals balances into interpreter.Balances ([]BalanceRow) and metadata into AccountsMetadata ([]AccountMetadataRow). A file accepted by this schema will therefore fail at runtime with a JSON unmarshal error, while the currently supported array form is marked invalid by the unversioned schema.
| @@ -83,35 +83,18 @@ | |||
| }, | |||
|
|
|||
| "Balances": { | |||
There was a problem hiding this comment.
🟠 [major] Keep the specs schema aligned with test specs
This changes the default specs schema's Balances definition to an object map, but the specs runner still decodes balances, expect.endBalances, and related fields as interpreter.Balances slices. Specs that validate against specs.schema.json will fail to load in the current runner, and existing valid specs that do not opt into v1.specs.schema.json will be reported as invalid by the default schema.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #171 +/- ##
==========================================
+ Coverage 70.71% 70.75% +0.03%
==========================================
Files 57 57
Lines 5262 5262
==========================================
+ Hits 3721 3723 +2
+ Misses 1328 1327 -1
+ Partials 213 212 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
On the previous PRs we refactored the specs and inputs schema by doing breaking changes. The change isn't realised yet so it still doesn't break user codebases (and we are still in time to publish utilities to update the schema) but it will break editor diagnostics that use the $schema field.
Fixed it by adding a v1 prefix in the new format, so that we can leave the old format in the repo and user will be able to keep referring to that
Pr can be read commit by commit