Skip to content

[ErrorHandler] Added type declarations where possible #32812

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
Jul 31, 2019

Conversation

tweichart
Copy link

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 -

I've added type declarations where possible (method parameters & return types). DocBlocks were replaced too if the type declarations made them redundant.
Void returns were replaced whenever necessary.
Also on one occasion the setting of the exceptionHandler was replaced by a function call, which did practically the same thing.

@tweichart
Copy link
Author

tweichart commented Jul 30, 2019

php ./phpunit src/Symfony/Component/HttpKernel/ is running fine locally, I actually don't know why this is failing at appveyor right now. Before the last commit the test was fine, I highly doubt that the last commit did anything concerning the HttpKernel test. Apart from the fact that it just reverted things to the way they were before the PR...
edit: seems fixed, thanks

@yceruto
Copy link
Member

yceruto commented Jul 30, 2019

could you please also remove the useless null default here:

private static $toStringException = null;

@tweichart
Copy link
Author

various checks seem fine for me locally:

$> php ./phpunit src/Symfony/Bundle/SecurityBundle/
#...
OK (231 tests, 679 assertions)
$> docker run -it --rm --name spu -v "$PWD":/usr/src/spu -w /usr/src/spu php:7.2.19-cli php ./phpunit src/Symfony/Component/HttpClient
#...
OK, but incomplete, skipped, or risky tests!
Tests: 245, Assertions: 511, Skipped: 5.
$> php ./phpunit src/Symfony/Component/ErrorHandler/
#...
OK (64 tests, 196 assertions)

php version 7.2.20

@yceruto
Copy link
Member

yceruto commented Jul 30, 2019

@tweichart don't worry about AppVeyor's failure, it's not related to your changes.

yceruto added a commit that referenced this pull request Jul 31, 2019
…chart)

This PR was merged into the 3.4 branch.

Discussion
----------

[Debug] minor fix for wrong method name case

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

Minor replacement of wrong case for function name, needs to be fixed in 3.4 as hinten [here](#32812 (comment)).

Commits
-------

8b2d67b minor fix for wrong case
@nicolas-grekas nicolas-grekas force-pushed the component_errorHandler_44 branch from 3b952e1 to ab6aae1 Compare July 31, 2019 15:58
@nicolas-grekas
Copy link
Member

Thank you @tweichart.

@nicolas-grekas nicolas-grekas merged commit ab6aae1 into symfony:4.4 Jul 31, 2019
nicolas-grekas added a commit that referenced this pull request Jul 31, 2019
…obias Weichart)

This PR was squashed before being merged into the 4.4 branch (closes #32812).

Discussion
----------

[ErrorHandler] Added type declarations where possible

| 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        | -

I've added type declarations where possible (method parameters & return types). DocBlocks were replaced too if the type declarations made them redundant.
Void returns were replaced whenever necessary.
Also on one occasion the setting of the exceptionHandler was replaced by a function call, which did practically the same thing.

Commits
-------

ab6aae1 [ErrorHandler] Added type declarations where possible
@tweichart tweichart deleted the component_errorHandler_44 branch August 1, 2019 06:58
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.4 Oct 27, 2019
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.

6 participants