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

Introduce createFormatter Static Method for Formatter Logic #20014

Conversation

salehhashemi1992
Copy link
Contributor

@salehhashemi1992 salehhashemi1992 commented Oct 19, 2023

Introduce a new static method, createFormatter, which encapsulates the existing logic for creating a formatter based on locale, type, and pattern.

It helps adhere to the DRY principle.

Also add two missing @thows lines in PHPDoc comments.

@codecov
Copy link

codecov bot commented Oct 19, 2023

Codecov Report

Attention: 16 lines in your changes are missing coverage. Please review.

Comparison is base (5d8a96e) 48.96% compared to head (a1dba1d) 22.25%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##           master   #20014       +/-   ##
===========================================
- Coverage   48.96%   22.25%   -26.71%     
===========================================
  Files         445      445               
  Lines       42839    42031      -808     
===========================================
- Hits        20978     9356    -11622     
- Misses      21861    32675    +10814     
Files Coverage Δ
framework/helpers/BaseFormatConverter.php 0.00% <0.00%> (-11.49%) ⬇️

... and 240 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@salehhashemi1992 salehhashemi1992 marked this pull request as ready for review October 19, 2023 13:48
@bizley
Copy link
Member

bizley commented Oct 19, 2023

Let's make it private so there will be not reason to test again for intl extension insie and there is no need to have this method public.

@salehhashemi1992
Copy link
Contributor Author

@bizley Done

@samdark samdark merged commit e029988 into yiisoft:master Oct 19, 2023
48 of 50 checks passed
@samdark
Copy link
Member

samdark commented Oct 19, 2023

Thanks.

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.

None yet

3 participants