Remover Símbolos do Número de Telefone#188
Remover Símbolos do Número de Telefone#188antoniamaia merged 16 commits intobrazilian-utils:mainfrom
Conversation
|
Fiquei com algumas dúvidas e coloquei nos comentários mesmo como review, conforme forem respondendo, atualizo mais o pull request! |
Codecov Report
@@ Coverage Diff @@
## main #188 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 5 5
Lines 90 93 +3
=========================================
+ Hits 90 93 +3
|
camilamaia
left a comment
camilamaia
left a comment
There was a problem hiding this comment.
Muito obrigada pela contribuição!! Deixei alguns comentários com sugestões de mudanças. Também atualizamos nossa doc para deixar mais clara as instruções para as próximas.
Valeu demais!
|
@camilamaia Atualizei os locais corretos das documentações agora e resolvi os conflitos. Se tiver mais algum ponto, só pontuar que modifico por aqui ;) |
|
|
||
| ```python | ||
| >>> from brutils import remove_symbols_phone | ||
| >>> remove_symbols_phone('+55 (21) 2569-6969') |
There was a problem hiding this comment.
Pelo o quê eu entendi da tua implementação, ela não estaria tirando o + nem o (espaço), certo? Então aqui o resultado seria +55 21 25696969 ao invés de 552125696969 se entendi certo,
Pensando aqui, acho que seria legal tirar esses dois símbolos também. @antoniamaia o quê você acha? Daí na issue de retirar o código do país, teria que passar a string sem símbolo nenhum.
There was a problem hiding this comment.
Atualizei a implementação e adicionei mais casos de teste :)
|
Valeu @giovannism20 ! Muito obrigada. Fiquei com uma dúvida só sobre quais símbolos remover, deixei ali comentado no código :) No mais, tá show!! |
|
|
||
| # When there are extra symbols, it only removes the specified symbols | ||
| self.assertEqual( | ||
| remove_symbols_phone("(21) 99402-9275!"), "21994029275!" |
There was a problem hiding this comment.
Acham que faria sentido removermos ainda outros símbolos e realmente deixar apenas números ?
cc: @camilamaia @antoniamaia
There was a problem hiding this comment.
@giovannism20 o padrão que estamos seguindo para os outros utilitários é de remover apenas os símbolos de cada contexto.. até poderia fazer sentido, mas como estamos seguindo este padrão poderia causar confusão para quem já utiliza a biblioteca.
camilamaia
left a comment
camilamaia
left a comment
There was a problem hiding this comment.
Aeee, valeu demais, agora tá 10/10!
3 participants
Descrição
#187
Mudanças Propostas
Checklist de Revisão
Comentários Adicionais (opcional)
Issue Relacionada
Closes #187