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

onUncaughtException should accept \Throwable instead of \Exception #14738

Conversation

@janisbiz
Copy link
Contributor

janisbiz commented Jan 19, 2020

Hello!

  • Type: bug fix

In raising this pull request, I confirm the following (please check boxes):

  • I have read and understood the Contributing Guidelines?
  • I have checked that another pull request for this purpose does not exist.
  • I wrote some tests for this PR.

Small description of change:

As of support for php 7.x and introduction of \Throwable, Phalcon Debug method of onUncaughtException should accept \Throwable instead of \Exception

Thanks

@niden

This comment has been minimized.

Copy link
Member

niden commented Jan 19, 2020

@janisbiz can you rebase your branch please. It seems the checks did not pass also.

@janisbiz janisbiz force-pushed the janisbiz:fix-on-uncaught-exception-to-accept-throwable-instead-of-exception branch from e4e3e95 to 51ac99a Jan 19, 2020
@janisbiz

This comment has been minimized.

Copy link
Contributor Author

janisbiz commented Jan 19, 2020

@niden thanks for noting. As it turns out my local fork was from 2017, so everything was really outdated. Should be fine now.

@janisbiz janisbiz requested a review from niden Jan 19, 2020
@niden

This comment has been minimized.

Copy link
Member

niden commented Jan 19, 2020

Thank you @janisbiz Once the tests are done I will merge this. Could you also be so kind as to update the changelog ? This will be in the 4.0.3 section

@niden niden added this to Working on it in Phalcon Roadmap Jan 19, 2020
@codecov

This comment has been minimized.

Copy link

codecov bot commented Jan 19, 2020

Codecov Report

Merging #14738 into 4.0.x will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##            4.0.x   #14738   +/-   ##
=======================================
  Coverage   67.97%   67.97%           
=======================================
  Files         483      483           
  Lines      111435   111435           
=======================================
  Hits        75749    75749           
  Misses      35686    35686
@niden
niden approved these changes Jan 19, 2020
@niden niden merged commit 58c1142 into phalcon:4.0.x Jan 19, 2020
12 checks passed
12 checks passed
Build Phalcon Pecl Package Build Phalcon Pecl Package
Details
phpcs
Details
PHP 7.2 Test on ubuntu-latest PHP 7.2 Test on ubuntu-latest
Details
PHP 7.3 Test on ubuntu-latest PHP 7.3 Test on ubuntu-latest
Details
PHP 7.4 Test on ubuntu-latest PHP 7.4 Test on ubuntu-latest
Details
PHP 7.2 Test on macOS-latest PHP 7.2 Test on macOS-latest
Details
PHP 7.3 Test on macOS-latest PHP 7.3 Test on macOS-latest
Details
PHP 7.4 Test on macOS-latest PHP 7.4 Test on macOS-latest
Details
codecov/patch Coverage not affected when comparing 04a6207...4b62ab3
Details
codecov/project 67.97% remains the same compared to 04a6207
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@niden

This comment has been minimized.

Copy link
Member

niden commented Jan 19, 2020

Thank you @janisbiz

@niden niden moved this from Working on it to Implemented in Phalcon Roadmap Jan 23, 2020
@niden niden moved this from Implemented to Released in Phalcon Roadmap Jan 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Phalcon Roadmap
  
Released
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.