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

Extend Exception from Throwable #207

Merged
merged 1 commit into from
Jul 11, 2022

Conversation

gsteel
Copy link
Contributor

@gsteel gsteel commented Jul 11, 2022

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Related tickets None
Documentation None required
License MIT

What's in this PR?

Extends the Exception interface from Throwable

Why?

This stops static analysis from moaning about an invalid catch

Checklist

  • Updated CHANGELOG.md to describe BC breaks / deprecations | new feature | bugfix
  • Documentation pull request created (if not simply a bugfix)

@gsteel gsteel force-pushed the exception-should-be-throwable branch from af4d62f to 2c95f8d Compare July 11, 2022 13:16
@gsteel gsteel force-pushed the exception-should-be-throwable branch from 2c95f8d to 7c4c250 Compare July 11, 2022 13:18
@gsteel
Copy link
Contributor Author

gsteel commented Jul 11, 2022

Can the technical BC break be overlooked in this instance? I can't imagine a real-world situation where this would break usage of this lib ??

Copy link
Contributor

@dbu dbu left a comment

Choose a reason for hiding this comment

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

thanks for the PR. i don't think we have to worry about this "BC break", the interface is not supposed to be implemented from outside of this library.

what's more, if it would be possible to have an exception that does not implement Throwable, catching Throwable would not work with those exceptions, which would be problematic.

@dbu dbu merged commit 19b207f into php-http:master Jul 11, 2022
@dbu
Copy link
Contributor

dbu commented Jul 11, 2022

@gsteel gsteel deleted the exception-should-be-throwable branch July 11, 2022 14:12
gsteel added a commit to gsteel/akismet that referenced this pull request Feb 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants