Skip to content

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

Merged
merged 1 commit into from
Jan 3, 2019
Merged

add missing type declarations #145

merged 1 commit into from
Jan 3, 2019

Conversation

dbu
Copy link
Contributor

@dbu dbu commented Jan 2, 2019

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Related tickets -
Documentation -
License MIT

What's in this PR?

Went over the code again to find places where we can add type hints.

* @return Cookie|null
*
* @throws TransferException
*/
private function createCookie(RequestInterface $request, $setCookie)
private function createCookie(RequestInterface $request, string $setCookieHeader)
Copy link
Contributor

Choose a reason for hiding this comment

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

: ?Cookie?

Copy link
Contributor

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.

Copy link
Contributor Author

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 ?

Copy link
Member

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

Copy link
Contributor Author

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.

@dbu dbu added this to the v2.0.0 milestone Jan 3, 2019
@dbu
Copy link
Contributor Author

dbu commented Jan 3, 2019

okay if i merge this and continue in a new PR to upgrade to php 7.1?

@Nyholm
Copy link
Member

Nyholm commented Jan 3, 2019

Yes, go ahead.

@Nyholm Nyholm mentioned this pull request Jan 3, 2019
@Nyholm
Copy link
Member

Nyholm commented Jan 3, 2019

I added #148 about dropping PHP 7.0

@dbu dbu merged commit 6a2df0d into 2.x Jan 3, 2019
@dbu dbu deleted the more-typing branch January 3, 2019 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants