Skip to content

Improve vocab example per gregsdennis suggestions #782

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

Merged
merged 2 commits into from
Aug 20, 2019

Conversation

handrews
Copy link
Contributor

This is heavily based on @gregsdennis 's work in PR #756.

Hopefully it is now more clear why I went off and updated a bunch
of other things before coming back to this. What I found while
reviewing that PR is that the example wasn't great to begin with,
and in a few places was outright wrong. This, understandably,
led to some of the enhancements to the example not being
quite right either.

Most notably, the old example should not have been set up
as its own meta-schema. I can barely even figure out why
I must have done that, it makes so little sense 😕

The example now follows the best practices from the previous
section by showing separate general purpose and single-vocabulary
meta-schemas, and discusses how they are used together.

I also switched "startDate" to "minDate" so that the parallel in behavior
with "minLength", "minItems", etc. was more intuitive. And used pattern
in addition to format because of the changes to how format is handled
by default (another thing that wasn't done yet when @gregsdennis
originally wrote his PR).

This incorporates most of the ideas from an earlier PR by
gregsdennis, updated and enhanced by some clarifications and
enhancments that were made after that PR was originally written.

The example now follows the best practices from the previous
section by showing separate general purpose and single-vocabulary
meta-schemas, and discusses how they are used together.
Copy link
Member

@gregsdennis gregsdennis left a comment

Choose a reason for hiding this comment

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

Looks good.

May be a good idea to show a valid schema that uses this meta-schema just to drive home that this is a "schema that validates schemas." Not sure if it's necessary, though. Might be sufficient to do that on the "JSON Schema explained" site.

Copy link
Member

@Relequestual Relequestual left a comment

Choose a reason for hiding this comment

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

Broadly yes I think this explains things better.
I would prefer changing "core-app-example" to something like "new-vocab-example", otherwise it may be confused with the core vocabulary or something related to core... Let's avoid that.

...and fix inconsistency in the example name.  Oops.
@handrews
Copy link
Contributor Author

@Relequestual I changed the example name to something similar to what you suggested (and noticed an inconsistency which got fixed along the way so that's a bonus).

@gregsdennis adding a schema (instance for the meta-schema) might be a good idea but I'm getting bogged down trying to come up with one- there are few things I'm worse at than coming up with examples.

Since you both approved it, I'm going to go ahead and merge (the update for the example name was trivial). @gregsdennis if you want to add such a valid schema instance, I would be in favor of that. I also think we can get away without it and, as you note, put it on the web site.

@handrews handrews merged commit b30d80b into json-schema-org:master Aug 20, 2019
@handrews handrews deleted the new-examp branch August 22, 2019 01:37
@gregsdennis gregsdennis added clarification Items that need to be clarified in the specification and removed Type: Maintenance labels Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification Items that need to be clarified in the specification core
Development

Successfully merging this pull request may close these issues.

3 participants