Skip to content

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

Merged
jeromegamez merged 1 commit into
5.xfrom
remove-lib-phonenumber
Mar 15, 2021
Merged

Make giggsey/libphonenumber-for-php an optional dependency#578
jeromegamez merged 1 commit into
5.xfrom
remove-lib-phonenumber

Conversation

@jeromegamez
Copy link
Copy Markdown
Member

See #577

@codecov
Copy link
Copy Markdown

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
Copy Markdown

@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
Copy Markdown
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
Copy Markdown

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