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

Initial commit of schema definition for invalid updates. #1083

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

andrewpollock
Copy link
Contributor

@andrewpollock andrewpollock commented Mar 2, 2023

This is my first foray into writing a JSON schema, and I've basically done it using https://www.jsonschema.net/app/schemas/231529 so let me know if you see any issues with it (including things like $id)

Contributes to #771

Copy link
Member

@michaelkedar michaelkedar left a comment

Choose a reason for hiding this comment

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

A few comments as I read up on JSON schema :)

docs/invalid_update.schema.json Outdated Show resolved Hide resolved
Comment on lines 32 to 39
"reason": {
"type": "string",
"title": "The reason Schema",
"examples": [
"INVALID_JSON",
"DELETED"
]
}
Copy link
Member

Choose a reason for hiding this comment

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

Should this have an enum field for the valid reasons?

If so, we could probably remove the examples.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oooh good call, I didn't realise you could do that.

Comment on lines 14 to 15
"default": [],
"title": "The invalid_records Schema",
Copy link
Member

Choose a reason for hiding this comment

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

nit: most of these titles don't really seem useful, and I don't think they're a required field.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've improved on them.

"title": "The invalid_records Schema",
"items": {
"type": "object",
"title": "A Schema",
Copy link
Member

Choose a reason for hiding this comment

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

This title is particularly unhelpful :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yes, yes it is. Improved.

andrewpollock and others added 3 commits March 9, 2023 15:13
Defer to the consolidated examples putting everything together.
* Use better titles
* Specify the supported reasons as an enum
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.

None yet

3 participants