-
Notifications
You must be signed in to change notification settings - Fork 446
Introduce subtypes of InvalidArgumentException #809
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
Conversation
I am pretty alone here managing this package, but personally I think this is a good PR. There is no BC-break and I can see why the specific exceptions add value. @ruudk Since you have actively helped getting PHP8.4 support in, would you be willing to provide some feedback? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a good change to me, except for the typo in ModuleByZeroException class name. From what I can tell, this won't have a BC impact. The newly more specific exceptions that are thrown in Money.php still extend from \InvalidArgumentException
.
There is a PHPStan error. If someone is able to see what is causing this, and resolve it, I can merge it. Otherwise I will have a look myself, but I don't know exactly when I have time for that. |
@frederikbosch #810 may solve the PHPStan error |
Now this one can be rebased against current master and then merged. |
78e1980
to
3fb5ac9
Compare
Great, thanks for the work. Nice addition. |
This PR introduces subtypes of
InvalidArgumentException
.This improves the log analysis and allow users to rely on type instead of exception message when handling specific errors.