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

Implement $schema #55

Closed
Julian opened this issue Feb 1, 2013 · 12 comments
Closed

Implement $schema #55

Julian opened this issue Feb 1, 2013 · 12 comments
Labels
Enhancement Some new desired functionality
Milestone

Comments

@Julian
Copy link
Member

Julian commented Feb 1, 2013

No description provided.

@Julian Julian mentioned this issue Feb 22, 2013
@gazpachoking
Copy link
Contributor

I'm guessing you mean support in the plain validate function, right?

What should happen if a $schema isn't found in the schema, and a validator class isn't explicitly specified? Should we default to one, or error?

@Julian
Copy link
Member Author

Julian commented Feb 22, 2013

Right, in validate.

@validates should be modified to pull the id out of IValidator.META_SCHEMA too.

I think we should default to one. The next release will be 1.0.0 so we can now make that default be draft 4 I think.

@Julian Julian closed this as completed Feb 22, 2013
@Julian
Copy link
Member Author

Julian commented Feb 22, 2013

Uh. Whoops?

@Julian Julian reopened this Feb 22, 2013
@gazpachoking
Copy link
Contributor

I have something for this, just trying to get all the updates merged in to the draft 4 branch now and tests updated. Do you think we should just have extra keys in validators for the schema uris to look this up?

@gazpachoking
Copy link
Contributor

Okay, here's what I've got. I'm unsure if schema URIs should just be mixed in with the common names in the validators dict like I'm doing right now though.
gazpachoking@112f3d6

@Julian
Copy link
Member Author

Julian commented Feb 24, 2013

Ech yes. I don't know why I said we should get this done before merging, I didn't actually mean that 😦. Let's see here...

@gazpachoking
Copy link
Contributor

Oh, sorry if I was premature there. I was just trying get the updated tests into draft 4 branch before working on it again.

@gazpachoking
Copy link
Contributor

We can always go back and do the merge again if I've done it wrong.

@Julian
Copy link
Member Author

Julian commented Feb 24, 2013

No no :)... It looks fine, I just didn't mean to say "we need this
otherwise we can't merge the other thing".

@gazpachoking
Copy link
Contributor

Oh no, I didn't take it that way. It just felt right to implement $schema in the draft 4 branch, and I wanted to pull in the master changes/tests before they diverged any further.

@Julian
Copy link
Member Author

Julian commented Feb 24, 2013

Ah OK, good. So I'll take a look in a bit, looks more or less mergeable as is.

@Julian
Copy link
Member Author

Julian commented Feb 24, 2013

Merged in #59.

@Julian Julian closed this as completed Feb 24, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Some new desired functionality
Projects
None yet
Development

No branches or pull requests

2 participants