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

Consistent naming cont'd #840

Merged
merged 2 commits into from
Oct 14, 2021
Merged

Consistent naming cont'd #840

merged 2 commits into from
Oct 14, 2021

Conversation

cleary
Copy link
Contributor

@cleary cleary commented Sep 22, 2021

Hi @yaxu,
I've added a few more fixes into the naming that I missed originally -

I've separated a commit for consideration, which is to allow the use of 'M (capital M) as a Major shorthand. This hasn't been applied to any other Major chords though - keen for your thoughts, and happy to discard.

@cleary
Copy link
Contributor Author

cleary commented Oct 13, 2021

@yaxu I'm unsure what the check failures mean, is there a specific problem I need to deal with to get this accepted?

@yaxu
Copy link
Member

yaxu commented Oct 14, 2021

Hi @cleary, sorry I've been distracted the last month. I will have a bit more time from November and all the time in the world from December!

I think the test failures are probably unrelated to your changes, I've re-run them just now.

Looking at your changes, I think they break backward compatibility? Perhaps we could copy the old definitions to a separate part of the file and mark them as deprecated for a little while, or something like that?

@cleary
Copy link
Contributor Author

cleary commented Oct 14, 2021

@yaxu all good, I understand you're going through some significant upheaval, just wanted to make sure there was nothing more I needed to/could do :)

Looking at your changes, I think they break backward compatibility?

No, there are no chord names removed - any of the renames in the top set of definitions have the old naming added as an alias below. I was very careful about that :)

@yaxu yaxu merged commit 38b29c7 into tidalcycles:main Oct 14, 2021
@yaxu
Copy link
Member

yaxu commented Oct 14, 2021

Ok thanks!

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.

2 participants