-
Notifications
You must be signed in to change notification settings - Fork 53
add missing type declarations #145
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
Conversation
* @return Cookie|null | ||
* | ||
* @throws TransferException | ||
*/ | ||
private function createCookie(RequestInterface $request, $setCookie) | ||
private function createCookie(RequestInterface $request, string $setCookieHeader) |
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.
: ?Cookie
?
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 we'd need PHP 7.1 for that. Is there any reason why 2.x is php 7.0+ and not 7.1+. 7.0 is EOL already.
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.
thats why i did not do it. but i wondered the same. happy to move to 7.1+. wdyt @Nyholm ?
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 agree. Lets do PHP7.1
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.
cool! however, i'd prefer to do that in a separate pull request, as it will entail quite a lot of changes.
okay if i merge this and continue in a new PR to upgrade to php 7.1? |
Yes, go ahead. |
I added #148 about dropping PHP 7.0 |
What's in this PR?
Went over the code again to find places where we can add type hints.