Skip to content

Conversation

linconkusunoki
Copy link
Contributor

related: #146

@codecov
Copy link

codecov bot commented Sep 25, 2020

Codecov Report

Merging #155 into main will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##              main      #155   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           51        51           
  Lines          938       955   +17     
  Branches       113       122    +9     
=========================================
+ Hits           938       955   +17     
Impacted Files Coverage Δ
src/utilities/index.ts 100.00% <100.00%> (ø)
src/utilities/phone/index.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8a45427...400c0f1. Read the comment docs.

Copy link
Contributor

@pedrovsp pedrovsp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you could add a few more test cases, I suggested two examples in the test file.

@hyanmandian
Copy link
Member

Hello nice MR @linconkusunoki and thanks @pedrovsp for the review! Another thing that I think could be improved is the removal of the config param... I know that I've suggested this but looking at the code and the test cases, maybe its better to just format the prefix when receiving this prefix. Something like:

format('5133333333') // (51) 3333-3333
format('555133333333') // +55 (51) 3333-3333

What do you think about it?

@linconkusunoki
Copy link
Contributor Author

hey guys thanks for the feedback and sorry for the late, I'll work on these changes

@rfoel
Copy link
Contributor

rfoel commented Oct 19, 2020

I like the idea of receiving the phone number and format the country code if it is present (eg. length > 11).

@hyanmandian
Copy link
Member

Hello guys, do you need some help?

@linconkusunoki
Copy link
Contributor Author

hey @hyanmandian for me it's ok if anyone wants to get this task #186, maybe close this PR and open another one?

@hyanmandian
Copy link
Member

Fala @linconkusunoki tudo certim? Entao, to fazendo um refactoring na lib e vou incluir essa mudanca la!

#398

Sinta-se a vontade a fazer o review se quiser! Assim q eu terminar o refactoring, vou te adicionar como contribuidor!

@hyanmandian hyanmandian mentioned this pull request Jan 26, 2025
39 tasks
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.

4 participants