Skip to content

Another tone notation#21

Open
zsinx6 wants to merge 5 commits intogciruelos:masterfrom
zsinx6:another-tone-notation
Open

Another tone notation#21
zsinx6 wants to merge 5 commits intogciruelos:masterfrom
zsinx6:another-tone-notation

Conversation

@zsinx6
Copy link
Copy Markdown

@zsinx6 zsinx6 commented Jul 17, 2018

No description provided.

@zsinx6
Copy link
Copy Markdown
Author

zsinx6 commented Oct 2, 2018

@gciruelos can you merge this?

@gciruelos
Copy link
Copy Markdown
Owner

thank you for pinging me, whenever I don't respond please do

Please remove accents from the note names, let's keep everything in English.
Also please give the variables of the new class a better name, because Do Re Mi... are not letters

@zsinx6
Copy link
Copy Markdown
Author

zsinx6 commented Oct 2, 2018

thank you for pinging me, whenever I don't respond please do

Please remove accents from the note names, let's keep everything in English.
Also please give the variables of the new class a better name, because Do Re Mi... are not letters

Since I'm sub classing the Letter class, when renaming the variables, everything must be re-implemented. Or am I missing something?

@gciruelos
Copy link
Copy Markdown
Owner

Yes, I think in this case refactoring the Letter class might be the easiest.

Comment thread musthe/musthe.py
"""
Another tone naming.

There are still 7: Dó, Ré, Mi, Fá, Sol, Lá, and Si.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

since these names are language-dependent, consider using locale, reading LC_* env, or accepting a lang option to get those name in the desired locale?

@gciruelos
Copy link
Copy Markdown
Owner

Please resolve conflicts

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.

3 participants