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

add isResource assertion. #222

Merged
merged 1 commit into from
Apr 13, 2017
Merged

add isResource assertion. #222

merged 1 commit into from
Apr 13, 2017

Conversation

abacaphiliac
Copy link
Contributor

i had to update the doc-block on propertiesExist so that the generated docs for that method were not incorrectly updated to mixed.

@rquadling
Copy link
Contributor

All $values are mixed.

It is upto the assertion to validate the type.

I'm also thinking propertiesExist should be deprecated in favour of the default allPropertyExists method.

@abacaphiliac
Copy link
Contributor Author

abacaphiliac commented Apr 12, 2017

i had to update the second param ($properties) of propertiesExist from mixed to array, not $value/$values. $properties is an array of property names. never a single property.

normally i wouldn't blend two things like this into a single request, but when i ran the doc generator script the docs for propertiesExist were updated also, and i think incorrectly. changing properties to array seems correct per the functionality and ensures that there are no errant changes to the docs.

regarding deprecation, i think the names of propertyExists and propertiesExist are too similar and leads to confusion.

propertyExists allows you to assert that a single object has a single property, while allPropertyExists allows you to assert that multiple objects all have a single property.

propertiesExist allows you to assert that a single object has multiple properties, while allPropertiesExists allows you to assert that multiple objects all have multiple properties.

@rquadling
Copy link
Contributor

With regard to the 'mixed'=>'array' docblock correction, I've done that manually.

@rquadling rquadling closed this Apr 13, 2017
@rquadling rquadling reopened this Apr 13, 2017
@rquadling rquadling merged commit c033c93 into beberlei:master Apr 13, 2017
@abacaphiliac
Copy link
Contributor Author

@rquadling thank you for fixing that. sorry for lumping that into this thread : )

@rquadling
Copy link
Contributor

No problem.

@rquadling
Copy link
Contributor

Can you cast your eye over #219.

@abacaphiliac
Copy link
Contributor Author

i agree with the changes in #219

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.

2 participants