-
-
Notifications
You must be signed in to change notification settings - Fork 61
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
Create JSON Schema for YAML File #192
base: main
Are you sure you want to change the base?
Conversation
…, move description to root of `parent` schema
The specification is not fully up to date as the fields added in #102 ( |
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.
I found a few mismatches with basic.yml
. I think that before merging this, we should at a test that checks whether basic.yml
(which is normative) passes the schema. We can do this using jsonschema
.
hayagriva.schema.json
Outdated
"title": { | ||
"type": "string", | ||
"description": "The title of the item.", | ||
"examples": [ | ||
"Rick Astley: How An Internet Joke Revived My Career" | ||
] | ||
}, |
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.
Does not support the object mode:
title:
value: "ZygOS: Achieving Low Tail Latency for Microsecond-Scale Networked Tasks"
verbatim: true
# also possible: sentence-case, title-case, short
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.
Do sentence-case
and title-case
still need to be handled? I just saw that they were removed for any formattable string in 0.4.0
. Is that still the case?
If so, basic.yml
might need updating, as it lists certain title
entries with those fields.
hayagriva/tests/data/basic.yml
Line 42 in edec836
sentence-case: There’s a lot we still don’t know about Libra |
There's also an entry in basic.yml
that uses a translation
field. Is that still in use?
hayagriva/tests/data/basic.yml
Line 227 in edec836
translation: Renaissance, The Unrooted |
Feel free to point me towards somewhere in the codebase where I could investigate myself; It's been a while since I've worked on this and don't want to be too burdensome with questions.
And remember upload it to schemastore 😄 |
I'd like to contribute to this PR, since I like Hayagriva and am currently working with it for my thesis. How could I help? |
We don't have a confirmation yet as to whether these changes will be merged or whether further modifications could be needed, so we can't guarantee a speedy merge, but if you're willing to pick up this work, you're free to address the reviews in your own branch and submit a new PR. |
Finally got back around to working on this! There are three things on my mind right now w.r.t. improving the schema so that it can be published. Person FieldsDo all fields referring to individuals ( If so, I can reference a common subschema for fields of this type. Affiliated RolesAfter working on making the values of the Is this the case? TestsI added tests for validating the schema itself, as well as validating Let me know if any of that should be changed to comply with the package's architecture. |
I took a pass at creating a JSON schema based on the Hayagriva specification. This will likely close #33!
Please provide any and all feedback! This is my first time creating a JSON schema.
This PR might be related to issue #134.
Fixes #206, #33