Skip to content

[HttpKernel] [5.0] Replace docblocks by type-hints #32276

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

Closed
wants to merge 0 commits into from
Closed

[HttpKernel] [5.0] Replace docblocks by type-hints #32276

wants to merge 0 commits into from

Conversation

tigitz
Copy link
Contributor

@tigitz tigitz commented Jun 29, 2019

Q A
Branch? master
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets continuation of #24722 and checks for #32179
License MIT
Doc PR N/A

This PR replaces docblocks by type hints in the HttpKernel component considering #32179. Some docblocks without valuable information got also removed.

@fabpot
Copy link
Member

fabpot commented Jul 3, 2019

Is it still a Draft PR?

@tigitz
Copy link
Contributor Author

tigitz commented Jul 3, 2019

@fabpot Hello, yes, tests are failing I need to invest some time to figure it out why

@@ -197,7 +197,7 @@ public function write(Request $request, Response $response)
$entry[1]['vary'] = [''];
}

if ($entry[1]['vary'][0] != $vary || !$this->requestsMatch($vary, $entry[0], $storedEnv)) {
if ($entry[1]['vary'][0] != $vary || !$this->requestsMatch($vary ?? '', $entry[0], $storedEnv)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's a similar test here:

if ($this->requestsMatch(isset($entry[1]['vary'][0]) ? implode(', ', $entry[1]['vary']) : '', $request->headers->all(), $entry[0])) {

@tigitz tigitz marked this pull request as ready for review July 9, 2019 13:28
@tigitz
Copy link
Contributor Author

tigitz commented Jul 9, 2019

Failing tests are unrelated

@tigitz
Copy link
Contributor Author

tigitz commented Aug 3, 2019

@nicolas-grekas @derrabus

Whenever you've got time for another review, thanks in advance.

@nicolas-grekas
Copy link
Member

Thank you @tigitz.

nicolas-grekas added a commit that referenced this pull request Aug 8, 2019
…ippe Segatori)

This PR was merged into the 5.0-dev branch.

Discussion
----------

[HttpKernel] [5.0] Replace docblocks by type-hints

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | continuation of #24722 and checks for  #32179
| License       | MIT
| Doc PR        | N/A

This PR replaces docblocks by type hints in the HttpKernel component considering #32179. Some docblocks without valuable information got also removed.

<!--
Replace this notice by a short README for your feature/bugfix. This will help people
understand your PR and can be used as a start for the documentation.

Additionally (see https://symfony.com/roadmap):
 - Bug fixes must be submitted against the lowest maintained branch where they apply
   (lowest branches are regularly merged to upper ones so they get the fixes too).
 - Features and deprecations must be submitted against branch 4.4.
 - Legacy code removals go to the master branch.
-->

Commits
-------

9e570a2 [Http-Kernel][5.0] Add type-hints
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.

5 participants