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

Make giggsey/libphonenumber-for-php an optional dependency #578

Merged
merged 1 commit into from
Mar 15, 2021

Conversation

jeromegamez
Copy link
Member

See #577

@codecov
Copy link

codecov bot commented Mar 15, 2021

Codecov Report

Merging #578 (e7e0b50) into 5.x (7c9ce86) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##                5.x     #578   +/-   ##
=========================================
  Coverage     92.73%   92.74%           
- Complexity     1578     1579    +1     
=========================================
  Files           150      150           
  Lines          3745     3747    +2     
=========================================
+ Hits           3473     3475    +2     
  Misses          272      272           

@acasademont
Copy link

@jeromegamez looks good to me, that was fast! I wasn't sure if you preferred to throw an exception if the library was not installed, but given the size of the library, I think it makes sense to make it 100% optional.

@jeromegamez
Copy link
Member Author

An exception would have caused a breaking change and I'm not ready for a new major release yet 😅

Some might argue that this is also a breaking change, when a project uses the library as a transient dependency, but I already have a response prepared for that 😬

I'll merge the PR and create a new minor release soon 💪

@acasademont
Copy link

Awesome, thanks :D

@jeromegamez jeromegamez merged commit 57884d4 into 5.x Mar 15, 2021
@jeromegamez jeromegamez deleted the remove-lib-phonenumber branch March 15, 2021 19:35
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