-
-
Notifications
You must be signed in to change notification settings - Fork 103
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
Add JSON schema for shard.yml
#623
Add JSON schema for shard.yml
#623
Conversation
Co-authored-by: Sijawusz Pur Rahnama <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work.
It would be nice to see how it behaves with existing shard.yml
files, for large public projects (e.g. lucky), and if it catches some errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Is there anything else that needs to be done for this? |
"dependencies": { | ||
"type": "object", | ||
"title": "dependencies", | ||
"description": "A list of required dependencies\nhttps://github.com/crystal-lang/shards/blob/master/docs/shard.yml.adoc#dependency-attributes", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thought (non blocking): I'm wondering if we could host a HTML rendering of the shard.yml
spec at a stable place under https://crystal-lang.org and link to that page instead of a github file that may be moved 🤔
poke @straight-shoota
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, we should certainly do that. Would fit right next to https://crystal-lang.org/reference/1.11/man/shards
Not sure why we're missing it there 🤔
Anyway, we can merge this as is and update the URL later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, seems like I had this idea a long time ago already: crystal-lang/crystal-book#476
But we haven't managed to make the format compatible with HTML rendering: #488
shard.yml
Implements #622. I can add automated tests if desired. After this, I can add it to JSON Schema Store.
I wasn't sure if we should restrict additional properties in the file, as there may be other tools that add stuff to the shard.yml. Off the top of my head, I can't remember any though, and they should probably move those kinds of things to their own config file.
Also if you're using VSCode, you can add this to the
.vscode/settings.json
and it will use the schema to vaidate the shard.yml that shards uses. I used this for testing.