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

Alteração do nome dos métodos parse para remove_symbols #112

Merged
merged 4 commits into from
Jun 27, 2023
Merged

Alteração do nome dos métodos parse para remove_symbols #112

merged 4 commits into from
Jun 27, 2023

Conversation

antoniamaia
Copy link
Contributor

@antoniamaia antoniamaia commented Jun 25, 2023

  • Alteração de cpf.parse para cpf.remove_symbols
  • Alteração de parse_cpf para remove_symbols_cpf
  • Alteração de cnpj.parse para cnpj.remove_symbols
  • Alteração de parse_cnpj para remove_symbols_cnpj
  • Atualização da documentação

@antoniamaia antoniamaia marked this pull request as draft June 25, 2023 10:38
parse("12.345.678/0001-90")
mock_sieve.assert_called()
def test_remove_symbols(self):
with patch("brutils.cnpj.remove_symbols") as mock_remove_symbols:
Copy link
Member

Choose a reason for hiding this comment

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

Aqui você quer verificar se o sieve foi chamado, então o mock tem que ser do sieve e nao do proprio remove_symbols.

porque no teste, vc quer testar que ao chamar o remove_symbols, o sieve será chamado.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

perfeito, Camila! Obrigada pela ajuda ❤️

@codecov
Copy link

codecov bot commented Jun 25, 2023

Codecov Report

Merging #112 (47ccd0f) into main (5ad023f) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #112   +/-   ##
=======================================
  Coverage   98.46%   98.46%           
=======================================
  Files           3        3           
  Lines          65       65           
=======================================
  Hits           64       64           
  Misses          1        1           
Impacted Files Coverage Δ
brutils/__init__.py 100.00% <ø> (ø)
brutils/cnpj.py 97.05% <100.00%> (ø)
brutils/cpf.py 100.00% <100.00%> (ø)

@antoniamaia antoniamaia marked this pull request as ready for review June 25, 2023 11:06
@antoniamaia
Copy link
Contributor Author

closes #110

Copy link
Member

@camilamaia camilamaia left a comment

Choose a reason for hiding this comment

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

Quanto ao código, tá 💯. Arrasou!

A documentação que foi alterada é da versão 1.0.1, da versão antiga. A ideia seria a gente não mudar nada da versão anterior para não confundir quem já utiliza a biblioteca. Além disso, o método que foi renomeado foi o parse e não o sieve. Então o correto seria alterar a documentação nos READMEs (pt-br e en), mudando o parse para remove_symbols. Faz sentido?

@camilamaia
Copy link
Member

camilamaia commented Jun 26, 2023

Ah, aqui também daria para deixar o título e a descrição do PR um pouco mais explícitos. Por exemplo:

Título
Alteração do nome dos métodos `parse` para `remove_symbols`

Descrição

* Alteração de `cpf.parse` para `cpf.remove_symbols`
* Alteração de `parse_cpf` para `remove_symbols_cpf`
* Alteração de `cnpj.parse` para `cnpj.remove_symbols`
* Alteração de `parse_cnpj` para `remove_symbols_cnpj`
* Atualização da documentação

Closes #110

Ah, e sempre dá para editar as informações do PR depois

image

image

@antoniamaia antoniamaia changed the title Renomear métodos parse para algum nome que seja mais explícito Alteração do nome dos métodos parse para remove_symbols Jun 26, 2023
@antoniamaia
Copy link
Contributor Author

antoniamaia commented Jun 26, 2023

Obrigada, @camilamaia! Você é perfeita ❤️ estudar com você é sensacional, tudo muito claro e bem explicadinho! Acredito que agora, finally closed #110? 🙌🏼

Copy link
Member

@camilamaia camilamaia left a comment

Choose a reason for hiding this comment

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

Mazaaaahhhh🏅

@camilamaia camilamaia merged commit 1b12ebf into brazilian-utils:main Jun 27, 2023
8 checks passed
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

2 participants