azuyalabs / yasumi

The easy PHP Library for calculating holidays
https://www.yasumi.dev
Other
1.04k stars 152 forks source link

Add new exceptions in Yasumi. #95

Closed qneyrat closed 5 years ago

qneyrat commented 6 years ago

@stelgenhof you prefer me in author or you ?

stelgenhof commented 6 years ago

@qneyrat You can put yourself as an author in the Class comments section :) and leave the file comments as is.

qneyrat commented 6 years ago

To catch specific exception and not invalidArgument is so generic. I want to be able to catch only UnknownIso31662Exception for example.

qneyrat commented 6 years ago

@stelgenhof are you ready to merge ?

stelgenhof commented 6 years ago

@qneyrat I believe there are some unanswered questions in the review of your PR.

rmasclef commented 6 years ago

@stelgenhof which ones ?

stelgenhof commented 6 years ago

@rmasclef There is a small conversation regarding the use of the UnknownIso31662Exception (https://github.com/azuyalabs/yasumi/pull/95/files/189127ef36697c2e230900d505b532f8efa8b6e6#diff-76dc48f17f4e7918c5844a2c6960d00cL189).

To me it looks like it is not concluded :)

rmasclef commented 6 years ago

@stelgenhof indeed, I agree with that one :+1:

stelgenhof commented 6 years ago

@qneyrat Can you please review your PR as there are some unanswered questions? If I don't get any feedback by the end of next week, I will close this PR.

qneyrat commented 6 years ago

it's ready for me :)

qneyrat commented 5 years ago

Hello, I don't have time to improve this PR for so that it is ready to merge. Anyone like to improve code? Else, I can close that.

stelgenhof commented 5 years ago

@qneyrat No worries. I will pick up from here. Anyways, thanks for the contribution!