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

Fix: Mark check() and coerce() as deprecated #476

Merged
merged 1 commit into from Dec 31, 2017

Conversation

localheinz
Copy link
Contributor

This PR

  • marks Validator::check() and Validator::coerce() as deprecated

@@ -217,7 +217,7 @@ if (isset($arOptions['--dump-schema'])) {

try {
$validator = new JsonSchema\Validator();
$validator->check($data, $schema);
$validator->validate($data, $schema);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably makes sense to use Validator::validate() here.

@@ -6,7 +6,7 @@

// Validate
$validator = new JsonSchema\Validator();
$validator->check($data, (object) array('$ref' => 'file://' . realpath('schema.json')));
$validator->validate($data, (object) array('$ref' => 'file://' . realpath('schema.json')));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably makes sense to use Validator::validate() here, too.

@localheinz localheinz force-pushed the fix/deprecate branch 2 times, most recently from d54234b to 3b34779 Compare December 30, 2017 18:50
@@ -71,6 +71,6 @@ public function testInvalidArgumentException()
{
$v = new Validator();
$this->setExpectedException('\JsonSchema\Exception\InvalidArgumentException');
$v->check(json_decode('{}'), json_decode(''));
$v->validate(json_decode('{}'), json_decode(''));
Copy link
Collaborator

Choose a reason for hiding this comment

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

You cannot use validate() in this situation, as the first argument is a reference. If you want to change this, you'll need to decode the document into an intermediate variable first, and then pass the variable.

@@ -36,7 +36,7 @@ public function testBadAssocSchemaInput()
$validator->validate($data, $schema);
}

public function testCheck()
public function testDeprecatedCheckStillValidates()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Method name needs clarification - validation in this test is supposed to fail, but the new name falsely implies that success is the intended outcome.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about I rename both to test.*DelegatesToValidate()?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That works :-). I'm not particularly fussed what they are named, as long as they're not ambiguous and give a reasonable idea of the intended purpose.

@localheinz
Copy link
Contributor Author

@erayd

Updated!

@bighappyface bighappyface merged commit 7d7f43a into justinrainbow:master Dec 31, 2017
@localheinz localheinz deleted the fix/deprecate branch January 1, 2018 00:55
@localheinz
Copy link
Contributor Author

Thank you, @bighappyface and @erayd!

@erayd erayd mentioned this pull request Feb 14, 2018
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