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

project update from 24Z WIMU #5

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kbogumil01
Copy link

Hi, here's our PR containing changes performed in 24Z semester.
Please check the 24Z_update.md to view more info.
We have updated the miditok library to 3.0.4 and because of this, the mixed run between current backend and new frontend (or opposite) is not possible. There have been changes made by the miditok creators, that forced us to re-implement and change some things like types names or token-note association logic.
Feel free to contact us if you need.

Br,
Karol

@justleon
Copy link
Owner

Hi :)

Thanks for the pull request. At first glance, the new front-end looks nice, and you can feel the improvement compared to the previous version. Moving the handling of individual tokenizers to one method looks good; I assume that had to be done because of the library update? It also somewhat reduces boilerplate, so it seems fine, although I'm unsure if the individual tokenizer classes are still needed. I'm not a front-end expert, so I don't know if I'll notice anything specific, but it's great that you've separated the components into CSS files.

I'll jump into Railway when I have free time and play around with your improvements. I'll likely finish the PR anyway—it'll be there for future generations.

Cheers,
Łukasz

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