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

Make the enum element comparison strict #132

Merged
merged 5 commits into from
Mar 27, 2015
Merged

Make the enum element comparison strict #132

merged 5 commits into from
Mar 27, 2015

Conversation

loucho
Copy link
Contributor

@loucho loucho commented Mar 6, 2015

The current check is matching numeric strings that are not equal ( like "604.1" == "604.10" because of PHP's == behavior that converts numeric strings to numbers and then performs a numeric comparison), even if type is checked before the comparison, the value should be identic

The current check is matching numeric strings that are not equal ( like "604.1" == "604.10" because of PHP's == behavior that converts numeric strings to numbers and then performs a numeric comparison), even if type is checked before the comparison, the value should be identic
[Proposal] new constraints name - for PHP7
@bighappyface
Copy link
Collaborator

@loucho have you reviewed the failing unit test in the build for this PR?

@loucho
Copy link
Contributor Author

loucho commented Mar 25, 2015

Oh, hadn't thought of that, i just checked it, seems my change fails when comparing an object (I assume that this is because objects are not === unless they are a reference to the same object in memory or something).

Maybe add a check for objects before making the comparison? what do you think?

@bighappyface
Copy link
Collaborator

@loucho sorry for the late response. Would you please rebase for a clean build?

loucho and others added 3 commits March 26, 2015 17:06
The current check is matching numeric strings that are not equal ( like "604.1" == "604.10" because of PHP's == behavior that converts numeric strings to numbers and then performs a numeric comparison), even if type is checked before the comparison, the value should be identic
@loucho
Copy link
Contributor Author

loucho commented Mar 26, 2015

Rebased and made the change, waiting on the CI tests

@bighappyface
Copy link
Collaborator

@loucho solid improvement. I am inclined to suggest wrapping the returns in curlies for consistency sake, and a good rebase would distill this PR down to a single new commit, but it's all good.

+1

I will merge this with another 👍 from the community.

Would you mind helping review some other PRs for their community 👍? I am sure they will return the favor 😄

#105 needs review.

@onlinesid
Copy link
Contributor

This is a bug fix so +1 !

@bighappyface
Copy link
Collaborator

Thanks for the review @onlinesid! Let's call this 1.4.1.

bighappyface added a commit that referenced this pull request Mar 27, 2015
Make the enum element comparison strict
@bighappyface bighappyface merged commit 2465fe4 into jsonrainbow:master Mar 27, 2015
@loucho
Copy link
Contributor Author

loucho commented Mar 27, 2015

Sure, will do

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants