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

Added an additional test for strict types validation #2699

Merged
merged 2 commits into from Apr 6, 2019

Conversation

Toflar
Copy link
Contributor

@Toflar Toflar commented Apr 6, 2019

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #2054
License MIT
Doc PR

Apparently this has been fixed in the meantime. I still added the test and fixed it (gettype() returns null uppercase).

@Toflar
Copy link
Contributor Author

Toflar commented Apr 6, 2019

Done at EUFOSSA Hackathon. Needs the label :)

Copy link
Member

@alanpoulain alanpoulain left a comment

Choose a reason for hiding this comment

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

Do you have an idea when it has been done?

@soyuka soyuka merged commit 6826437 into api-platform:2.4 Apr 6, 2019
@soyuka
Copy link
Member

soyuka commented Apr 6, 2019

Thanks @Toflar

@Toflar
Copy link
Contributor Author

Toflar commented Apr 6, 2019

Do you have an idea when it has been done?

I tried to track it down and it looks like it has actually never failed 😄 Here's the original build: https://travis-ci.org/api-platform/core/jobs/397805821
As you can see, the only thing that failed was that the null is not equal to NULL but the validation was actually correct 😄

@Toflar Toflar deleted the strict-types branch April 6, 2019 14:42
@dunglas
Copy link
Member

dunglas commented Apr 6, 2019

Some context about the initial PR: Han told me that strict types weren’t working with API Platform. I was sure that they were, so he opened a PR with a failing test 😂

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.

None yet

7 participants