Skip to content

Conversation

thewilkybarkid
Copy link
Contributor

Following the advice in jsonrainbow/json-schema#301 I've tried to update the library to v3's way of resolving references.

I've had to remove two tests as I don't think there's a way (as it stands) to make them pass. An upstream change to throw an exception would work, but it doesn't look to be a trivial change.

Copy link
Owner

@webmozart webmozart 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 in general!

$this->validator->validate((object) array('name' => 'Bernhard'), 12345);
}

/**
Copy link
Owner

@webmozart webmozart Oct 11, 2016

Choose a reason for hiding this comment

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

Why did you remove these tests? How does the validator behave now in these cases?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

get_object_vars() expects parameter 1 to be object, string given in justinrainbow/json-schema/src/JsonSchema/SchemaStorage.php:103. Don't think there's a way to fix it here.

composer.json Outdated
"require": {
"php": "^5.3.3|^7.0",
"justinrainbow/json-schema": "^2.0",
"justinrainbow/json-schema": "^3.0",
Copy link

@xabbuh xabbuh Oct 11, 2016

Choose a reason for hiding this comment

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

Is there a reason not to allow both versions 2.x and 3.x?

Copy link

Choose a reason for hiding this comment

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

since yesterday there is also a 4.x

Copy link
Owner

Choose a reason for hiding this comment

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

thanks for the hint @staabm! I think it would indeed be good to support all three branches. @thewilkybarkid could you adapt the PR?

@thewilkybarkid thewilkybarkid changed the title justinrainbow/json-schema version 3 justinrainbow/json-schema version 3/4 Nov 17, 2016
@thewilkybarkid
Copy link
Contributor Author

This now supports versions 2, 3 and 4, though it's getting quite messy internally.

@yp28
Copy link

yp28 commented Dec 5, 2016

@webmozart, can we get this merged? I just ran into an incompatibility issue and this would help me greatly.

@thewilkybarkid
Copy link
Contributor Author

Ping @webmozart.

I now have a schema that v2 appears to fail on, but I can't update due to this library (which is a dependency of Puli, I'm not actually using it myself).

@gsdevme
Copy link

gsdevme commented Jun 7, 2018

What is the latest here, is there something outstanding? @webmozart the unit tests are all passing

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.

6 participants