-
Notifications
You must be signed in to change notification settings - Fork 145
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 support for $defs
instead of definitions
.
#338
base: main
Are you sure you want to change the base?
Conversation
I did not inspect the code, but I believe this needs an implementation of the |
So, either this project is stuck at draft-07 and gradually fades to irrelevance, or someone takes on the changes needed to update it to 2020-12. Barring participation by the OP (which he has said he won't do), it seems to me like a piecemeal approach is more practical than a grand all-encompassing project. But maybe that's my too-practical side coming out. |
Piece-by-piece I think it's an ok approach. The thing that strikes me is that it would need a flag to check if it's draft 2020-12. I don't know exactly where that would be defined though, maybe that is not defined yet? |
Checking a flag is an excellent idea. I will see what I can figure out about that. Taking that as given, it would helpful to know if my trivial change in this PR is sufficient to support I also noticed the use of |
Just doing some checking, it seems like there is no flag currently. Two approaches occur to me: Read it from
|
I would say read it from schema if you did not provide a class with a specific validator that you want to override. I.e. have a factory that creates the schema-specific validator by parsing the key |
I'm for updating the library to the latest standard, even if incompatible with older drafts. Support for older versions can be maintained on branches, if needed. People should be encouraged to migrate their schema to newer versions of the standard. This PR is acceptable as is I think. No need for flagging. In #153 I created an issue for support 2020-12. And ask for volunteers in helping. Maybe the moment has come? To work on it as a team? |
In that case, perhaps it should check for Also, what about a test? Does it make sense to modify one or more of the existing ones or to create a new one? |
I don't know if supporting only one schema like that would be good, since it limits hacking and having to work on supporting each schema as a whole instead of gradually working on it with experimental support. I'm not sure which approach would be best right now for supporting multiple schemas, but here are a few:
Each of these would have their pros and cons for hacking and ease of developing, but I don't think any of them would have crucial flaws that the user would be blocked from hacking because we chose one style over another. I can start devoting some time now for this project, so I'll pick up the dangling PRs I had probably next month. |
In the context of this PR, at least, the change is backwards compatible with draft-07. Other features of 2020-12 will not be, of course. Then the issue could become more acute. |
One of the changes for the 1020-12 schema specification (#153) is the keyword
$defs
instead ofdefinitions
. This PR implements that change.With this change in place I can load the MNX schema and correctly validate MNX files against it. However, I am a newbie here, so I welcome comments on how to improve it.