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

Fix PHP7 error handler compatibility. #11518

Merged
merged 5 commits into from Dec 10, 2017

Conversation

Projects
None yet
4 participants
@dereuromark
Member

dereuromark commented Dec 7, 2017

Possible solution of #11514
We convert it back to the PHP5 exception where needed as the constructor for ExceptionRenderer exceptions Exception which is not compatible with PHP7.
Removing the typehint here can only be done in a non-patch version as the eco system could implement them still as such.

@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Dec 7, 2017

Codecov Report

Merging #11518 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #11518      +/-   ##
============================================
- Coverage     93.38%   93.36%   -0.02%     
- Complexity    13028    13034       +6     
============================================
  Files           436      436              
  Lines         32775    32848      +73     
============================================
+ Hits          30606    30669      +63     
- Misses         2169     2179      +10
Impacted Files Coverage Δ Complexity Δ
src/Error/Middleware/ErrorHandlerMiddleware.php 90.9% <100%> (ø) 26 <0> (ø) ⬇️
src/Http/Client/Message.php 0% <0%> (ø) 7% <0%> (+3%) ⬆️
src/Http/Client/Response.php 93.72% <0%> (+0.72%) 58% <0%> (+3%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e7f5da5...248bb55. Read the comment docs.

codecov-io commented Dec 7, 2017

Codecov Report

Merging #11518 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #11518      +/-   ##
============================================
- Coverage     93.38%   93.36%   -0.02%     
- Complexity    13028    13034       +6     
============================================
  Files           436      436              
  Lines         32775    32848      +73     
============================================
+ Hits          30606    30669      +63     
- Misses         2169     2179      +10
Impacted Files Coverage Δ Complexity Δ
src/Error/Middleware/ErrorHandlerMiddleware.php 90.9% <100%> (ø) 26 <0> (ø) ⬇️
src/Http/Client/Message.php 0% <0%> (ø) 7% <0%> (+3%) ⬆️
src/Http/Client/Response.php 93.72% <0%> (+0.72%) 58% <0%> (+3%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e7f5da5...248bb55. Read the comment docs.

@markstory

This comment has been minimized.

Show comment
Hide comment
@markstory

markstory Dec 7, 2017

Member

Any chance we could add a test for the scenario this is handling?

Member

markstory commented Dec 7, 2017

Any chance we could add a test for the scenario this is handling?

@ADmad

This comment has been minimized.

Show comment
Hide comment
@ADmad

ADmad Dec 9, 2017

Member

I added a test.

Member

ADmad commented Dec 9, 2017

I added a test.

@dereuromark

This comment has been minimized.

Show comment
Hide comment
@dereuromark

dereuromark Dec 10, 2017

Member

Thank you for helping with the test case :)

Member

dereuromark commented Dec 10, 2017

Thank you for helping with the test case :)

@dereuromark dereuromark merged commit 4b10e9c into master Dec 10, 2017

6 checks passed

codecov/patch 100% of diff hit (target 93.38%)
Details
codecov/project Absolute coverage decreased by -0.01% but relative coverage increased by +6.61% compared to e7f5da5
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
stickler-ci No lint errors found.

@dereuromark dereuromark deleted the bugfix/php7exceptions branch Dec 10, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment