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

Promote nullable types from PHP 7.1 #6715

Merged
merged 2 commits into from
Sep 21, 2017
Merged

Conversation

PowerKiKi
Copy link
Contributor

Also don't show type hinting in a bad light with sentences such as "If you insist on type-hinting"

Also don't show type hinting in a bad light with sentences such as "If you insist on type-hinting"
Also note that if you use type-hinting in your methods, i.e.
``setAddress(Address $address)``, you will have to specifically
allow null values, otherwise ``setAddress(null)`` will fail to
remove the association. Starting from PHP 7.1 you should use
Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't add the detail of PHP 7.1: just the fact that a nullable hint should be used

allow null values, otherwise ``setAddress(null)`` will fail to
remove the association. Starting from PHP 7.1 you should use
nullable types by prefixing the type with a ``?``,
``setAddress(?Address $address)``. Older PHP versions will only
Copy link
Member

Choose a reason for hiding this comment

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

We really don't care about older PHP versions here

@Ocramius Ocramius self-assigned this Sep 21, 2017
@Ocramius Ocramius added this to the 2.6.0 milestone Sep 21, 2017
Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Thanks @PowerKiKi!

@Ocramius Ocramius merged commit f96dc3b into doctrine:master Sep 21, 2017
@PowerKiKi PowerKiKi deleted the patch-3 branch September 21, 2017 12:30
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.

2 participants