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

Implementa escala cromática #5

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Luiz-Eduardo
Copy link

Esse PR busca implementar a escala cromática no notas_musicais.

O que foi feito

  • Implementei a escala cromática, adicionando a tonalidade na constante ESCALAS;
  • Implementei novos testes parametrizados no test_deve_retornar_um_erro_dizendo_que_a_escala_não_existe;

Pontos de discussão

  • Pensei em utilizar o type annotation Literal pra tentar garantir que as tonalidades enviadas pelo usuário seriam apenas as já implementadas, algo tipo:
from typing import Literal
Tonalidades = Literal['maior',  'menor', 'cromatica']

Mas acho que fugia do escopo do PR e talvez perdesse o sentido de alguns testes e validações como o KeyError, queria saber sua opinião sobre isso 🤔

  • Pensei sobre separar os testes parametrizados (em outro PR) deixando um teste pra cada tonalidade, porque ao longo do tempo provavelmente esse teste cresça bastante e dificulte a visibilidade do que ele está testando, o que acha?

Extras

  • Corrigi alguns typos encontrados no caminho perdão por manchar o PR mas o TOC não deixou passar batido, se quiser posso tirar esse commit daqui e abrir outro PR

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.

1 participant