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

[HttpFoundation] Add type-hints #33088

Merged
merged 1 commit into from Aug 11, 2019

Conversation

@azjezz
Copy link
Contributor

commented Aug 9, 2019

Q A
Branch? master
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #32179
License MIT

replace #32271

@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented Aug 9, 2019

Please revert all return-type changes, we're not ready to do them.

@azjezz azjezz force-pushed the azjezz:httpfoundation-type-hints branch 10 times, most recently from 1b9f1b6 to 4588228 Aug 9, 2019

@azjezz azjezz force-pushed the azjezz:httpfoundation-type-hints branch 7 times, most recently from 587f6fb to b0f575c Aug 9, 2019

@chalasr chalasr added this to the 5.0 milestone Aug 9, 2019

@azjezz azjezz force-pushed the azjezz:httpfoundation-type-hints branch from b0f575c to 7fa1d81 Aug 10, 2019

@azjezz azjezz force-pushed the azjezz:httpfoundation-type-hints branch 2 times, most recently from e2820cc to 36373fa Aug 10, 2019

@nicolas-grekas nicolas-grekas force-pushed the azjezz:httpfoundation-type-hints branch from 36373fa to b1be9ce Aug 11, 2019

@nicolas-grekas
Copy link
Member

left a comment

@azjezz up for a PR on 4.4 to take my comments into account? I squashes everything here and made some additional fixes too.

* @return self
*/
public static function fromString($headerValue)
public static function fromString(?string $headerValue)

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Aug 11, 2019

Member

because we were using it ourselves, we have to accept null unless we submit a deprecation on branch 4.4

@nicolas-grekas nicolas-grekas force-pushed the azjezz:httpfoundation-type-hints branch from b1be9ce to f449e38 Aug 11, 2019

nicolas-grekas added a commit that referenced this pull request Aug 11, 2019

minor #33112 [HttpFoundation] some cleanups (azjezz)
This PR was merged into the 4.4 branch.

Discussion
----------

[HttpFoundation] some cleanups

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

As found by @azjezz in #33088

Commits
-------

f62a3c0 [HttpFoundation] some cleanups

@nicolas-grekas nicolas-grekas force-pushed the azjezz:httpfoundation-type-hints branch 4 times, most recently from 4f4f37f to 61da067 Aug 11, 2019

@nicolas-grekas nicolas-grekas force-pushed the azjezz:httpfoundation-type-hints branch from 61da067 to ead419b Aug 11, 2019

@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented Aug 11, 2019

Thank you @azjezz.

@nicolas-grekas nicolas-grekas merged commit ead419b into symfony:master Aug 11, 2019

2 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details

nicolas-grekas added a commit that referenced this pull request Aug 11, 2019

minor #33088 [HttpFoundation] Add type-hints (julien57)
This PR was merged into the 5.0-dev branch.

Discussion
----------

[HttpFoundation] Add type-hints

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #32179
| License       | MIT

replace #32271

Commits
-------

ead419b add type-hints
@derrabus derrabus referenced this pull request Aug 12, 2019
57 of 57 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.