-
Notifications
You must be signed in to change notification settings - Fork 668
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
Migration/4.0 #1209
Migration/4.0 #1209
Conversation
Sorry, maybe it's late and I don't understand. :-) Are any of these fixes duplicated in my other PR about build process, and also my Font PR? I don't want to have us overwriting each others changes when a merge happens. |
For example, I also moved the legacy font tools out, and I also updated them to work again with the new Font PR branch. I anticipate some bad rebase / merges coming soon :-}. |
No this independent. You asked me to give it a go with Changes apart from including the path in the imports are minor in this PR. |
Hey if you have time can you try npm linking to my setFont branch? I just tested today's rebase over master, and it is showing no visual diffs. I'd like to see what public API things I may have broken, and which things my branch does better than the current master. Unfortunately I don't have a fancy app like yours to integrate with my branch. Your app is very nice! I'll try to some musicxml with it next week. Here's my branch: I'm currently reviewing your setFontMinor PR, and then I'll head to bed. |
I did it already, results are in #1200 have a look there :) you just made a commit, it should be no difference but I will give it a try also |
@0xfe ready to review |
We should try to solve the I need to do more research as I'm sure someone has developed a solution via a webpack plugin or something. In my font PR, I've consolidated your
into a single
Since your @ paths before were all pointed to the same module anyways. |
I left them separted to allow possible combinations (ie.: Brabura static and others dynamic) In any case this is not related to this PR. :) |
@0xfe Ready to review |
I'd recommend that this is the next PR to be reviewed and merged. I'm happy to be the last blocker for shipping a 4.0 beta. 🥇 hahah. But I think we are close. Rodrigo has been testing some of my recent builds with his PianoPlay project. |
Thanks for this. Looks good. Merging. (Is there a tslint plugin that will auto-order the imports?) |
Yes, we discussed last week and Rodrigo and I chose the "eslint-plugin-simple-import-sort": "^7.0.0". It has just a few options that you can set in .eslintrc. In my Fonts branch, I'm now using the config below. It will autofix imports, combine duplicates if possible, add newlines, and sort them based on your provided array of regex. I just used the default sort order, but moved the two imports that start with "vex***" to the top.
|
Includes relative path imports for test files from: 0xfe#1209
Includes relative path imports for test files from: 0xfe#1209
Issues 1 & 2 described in #1200 resolved:
3 needs still to be resolved
eslint warnings resolved