Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add $schema to JSON specs #631

Closed
wants to merge 3 commits into from
Closed

feat: add $schema to JSON specs #631

wants to merge 3 commits into from

Conversation

domoritz
Copy link
Member

@domoritz domoritz commented Dec 16, 2024

This enables autocomplete and add explicit versions to the examples.

@domoritz domoritz requested a review from jheer December 16, 2024 16:11
Copy link
Member

@jheer jheer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have mixed feelings about pegging all test specs to a specific version, as this creates an extra point of potential error, requiring regeneration of all examples with every version change. If we still want to include a schema, an alternative option would be to point to latest.

On the other hand, I can see why it might be nice to have a fixed schema in the published JSON on the website. Not sure that outweighs the other issues here, and we could potentially set schema separately solely for the online published examples.

Finally, the tests are now failing so obviously that needs to be resolved.

bin/prepare-examples.js Outdated Show resolved Hide resolved
@domoritz
Copy link
Member Author

Fixed the typo, tests, and added an explicit version variable.

Post 1.0, we could for example switch to const version = pkg.version.split('.').slice(0, 2).join('.'); so that the version only tracks the minor version but not the patch version. I could also change the tests to not test the $schema property if you prefer.

I don't feel strongly either way. Having auto complete in the specs in the repo is nice.

Screenshot 2024-12-16 at 12 39 26

@domoritz domoritz requested a review from jheer December 16, 2024 17:42
@jheer
Copy link
Member

jheer commented Dec 16, 2024

Fixed the typo, tests, and added an explicit version variable.

Post 1.0, we could for example switch to const version = pkg.version.split('.').slice(0, 2).join('.'); so that the version only tracks the minor version but not the patch version. I could also change the tests to not test the $schema property if you prefer.

I don't feel strongly either way. Having auto complete in the specs in the repo is nice.
Screenshot 2024-12-16 at 12 39 26

FWIW, the JSON specs are auto-generated from the YAML, so you probably shouldn't be editing those files anyway. (And for other files, you can of course add $schema if you like.) The YAML specs already have the schema and autocomplete applied if you are using VS Code, as the repo includes Code settings to apply the schema to the YAML specs.

@domoritz
Copy link
Member Author

Oh neat. I added a setting so we get autocomplete for json as well: #633. Happy to defer to your preference for this pull request.

@jheer
Copy link
Member

jheer commented Dec 16, 2024

Closing out in favor of #633 and to avoid maintenance issues around stale version numbers.

@jheer jheer closed this Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants