-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add explicit nullable types to PSR-17 #1321
Add explicit nullable types to PSR-17 #1321
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes LGTM 馃憤
Let's wait for @shadowhand review and call for a vote
1bcb9c8
to
796564d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess i am not allowed to vote, but I support the change.
I had previously verified that there is no complaint from PHP when the extending class does not yet have the ?
in its parameter type: https://3v4l.org/5TInO
Thank you for checking this David! |
@shadowhand we need you to call a vote to approve this, but before we need to fix php-fig/http-factory#16 by bumping to PHP 7.1+. Let me know how you want to proceed. |
I'm fine providing the updated PR if the author of the current one is not able to, cc @Ayesh 馃檪 |
I opened a PR to update the interface, its PHPDoc and bump minimal PHP version. If in the meantime the original author wants to update his PR, I'm totally OK to keep his 馃檪 |
The vote has been called. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Passed voting
In order to make this one pass 馃檪