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

4.0.0 #1155

Closed
ronyeh opened this issue Sep 25, 2021 · 54 comments
Closed

4.0.0 #1155

ronyeh opened this issue Sep 25, 2021 · 54 comments

Comments

@ronyeh
Copy link
Collaborator

ronyeh commented Sep 25, 2021

I'm going to open the discussion as a GitHub issue. When we ship it, we can close this issue and celebrate. :-)

I started a kanban here.
Edit: a trello style project board is probably too much overhead since we only have a few collaborators. I put up a wiki page instead: https://github.com/0xfe/vexflow/wiki/TODOs-for-4.0.0

Myke started a thread on the vexflow mailing list. We can monitor it for any replies: https://groups.google.com/g/vexflow/c/e61nXKB-dKo

Please list your "must-haves" and also "nice-to-haves" below. The rest can be punted to version 4.1.0, or version 5.0.0. We can re-organize any items if necessary on the project board: https://github.com/0xfe/vexflow/projects/4

@ronyeh
Copy link
Collaborator Author

ronyeh commented Sep 25, 2021

@mscuthbert @rvilarl @tommadams @0xfe

Please cc any others that we'd like to include on this discussion.

@mscuthbert started this disussion at #1149

Let's freeze any features soon-ish :-). Then we can just test and fix tiny bugs before we cut the 4.0.0 release.

@rvilarl
Copy link
Collaborator

rvilarl commented Sep 25, 2021

@ronyeh I do not know how to add cards to the project :(

Anyway I suggest:

  • clasify all the issues assign features and bugs to one release. Close rejected features.
  • agree on the level of minimum level of documentation that we want to achieve.

@ronyeh
Copy link
Collaborator Author

ronyeh commented Sep 26, 2021

Let me add those to the project board.

The project seems like a visual way to organize GitHub issues (like a basic version of Trello), so if there is a really important item to include in 4.0.0, I'd recommend turning it into a GitHub issue, and then I'll track it using the project kanban.

As for your two items:

  • Classify Issues / Triage. Let's identify any must have fixes and we can tag them for 4.0.0. I don't think we need to reject or close all the other issues though. We can just label them as future release. The future could be 4.1.0, or 5.0.0.... who knows.
  • Minimum level of documentation. That's a discussion that you can champion :-). I would love better documentation. However, my sense from our earlier discussion thread with @mscuthbert and @0xfe is that better docs is not an absolute must have for cutting a new release.

@mscuthbert
Copy link
Collaborator

For documentation the necessities to me are:

  1. how to install Vexflow for use in another project (npm install vexflow).
  2. loading it on a standalone website (<script src="https://cdn..../vexflow-latest.min.js"></script>).
  3. make sure that the EasyScore tutorial (https://github.com/0xfe/vexflow/wiki/Using-EasyScore) works in v4, since that's where the main website points people

numbers 1 and 2 are actually missing from all current versions. I think 3 is still good.

@tommadams
Copy link
Contributor

Thanks for the heads up! One feature that I would personally like to get in that feels like it belongs to a major point release is the ability to construct your own render context and pass it to the Renderer constructor (while preserving the existing constructor arguments for backwards compatibility of course).

I think all that is required is a light refactoring and moving some code from Renderer.resize into CanvasContext.

As for why... I've hacked together an SVGContext that generates a string rather than a DOM object, which could in theory allow Vexflow to run as a webworker without an offscreen canvas: it seems like widespread support for that feature is a long way off. Text measurement is a tricky issue but can technically be solved with a text measurer object that runs on the main thread and communicates with the webworker via messages.

What to folks think?

@ronyeh
Copy link
Collaborator Author

ronyeh commented Sep 27, 2021

:-) Send in a PR and we can all decide if it'll be in 4.0.0 or 5.0.0, haha. (Michael seems to be itching to get a 4.0.0 out 😃.) It sounds cool though. So this PathContext (or whatever you named it) just builds an internal string representation (or an array of strings, which is ultimately joined)?

It reminds me of the d3 path module, which I ran across a while back: https://github.com/d3/d3-path/blob/main/src/path.js

As long as the PR isn't too crazy, I don't see why not.

So let's say we'll allow Rodrigo, Ron, and Tom each the opportunity to include one last personal pet PR into the 4.0.0 release. Then we're feature frozen and can only test and fix minor GitHub issues until Mohit decides he wants to publish it to npm and unpkg. :-)

@ronyeh
Copy link
Collaborator Author

ronyeh commented Sep 27, 2021

A trello style project board is probably too much overhead since we only have a few collaborators. I put up a wiki page instead:

https://github.com/0xfe/vexflow/wiki/TODOs-for-4.0.0

It's probably safest if you open an issue and then just add a URL to that issue into the wiki for tracking purposes. I have no idea what happens if two people edit the wiki simultaneously, so be careful. :-)

@mscuthbert
Copy link
Collaborator

Yeah, I'm a release often type of person (at least philosophically -- some projects are too much of a pain to build a release for). And I know I'll ramp up my contributions to VF 4+ more once I'm seeing it every day in my projects. Pinning a release to master or another moving target isn't something I'm comfortable doing except in hobby projects.

@mscuthbert
Copy link
Collaborator

I think we've forgotten to tag @h-sug1no in this discussion who has contributed so much since the end of 1.2 also.

@0xfe
Copy link
Owner

0xfe commented Sep 27, 2021

+1 for @mscuthbert's comment about releasing early and often, and staying on HEAD. We don't need to be too rigorous here, get stuff into 4.0, and iterate fast to stabilize it.

@tommadams generating an SVG string sounds really useful for many reasons -- that PR is happily welcomed!

@h-sug1no
Copy link
Contributor

Thank you for notifying me!

Right now, I don't have any projects that depend on vexflow, so I don't have any special requirements for 4.0.0.

@mscuthbert
Copy link
Collaborator

v4.1/5 request that I'm willing to implement once this is out: <g> tags for more elements in svg -- find the clef inside the svg and change its color, etc. -- I find the <g> tags for stavenotes super valuable and wish we had more of them.

@sschmidTU
Copy link
Contributor

sschmidTU commented Sep 29, 2021

I find the <g> tags for stavenotes super valuable

Yes, also for other elements, especially when they have a class, e.g. <g class="vf-stavenote" id="vf-auto1143">.

In OSMD, we added this for TabNotes as well (class=vf-tabnote) with context.openGroup() and context.closeGroup() at the beginning and end (more or less) of draw(). It's useful for SVG manipulation like coloring, as mscuthbert said.
image
edit: this was already added for Tabnotes on master. I thought it wasn't when i didn't see the classes on the tests page.

@AaronDavidNewman
Copy link
Collaborator

same, the main modification I have in my branch for smoosic is to be able to put many elements into groups. For any application that's interactive, you need to know where everything is.

@ronyeh ronyeh added the 4.0 label Sep 30, 2021
@ronyeh
Copy link
Collaborator Author

ronyeh commented Oct 1, 2021

Awesome, it sounds like for version 4.1, we will have SVG group tags for everything! @mscuthbert can open a GitHub issue for this and @sschmidTU / @AaronDavidNewman can review the PR when it is submitted (post 4.0 release).

@rvilarl
Copy link
Collaborator

rvilarl commented Oct 7, 2021

I feel that we should set a deadline for 4.0. If some 4.0 are not ready by then, we can just delay them. What do you think? This weekend 10.10? That would be exiting!

@ronyeh
Copy link
Collaborator Author

ronyeh commented Oct 7, 2021

I'm out this weekend until mid next week. For a major revision I'd rather have the major contributors each give a thumbs up re: features and known bugs, than commiting to a particular date. :-) But that's my opinion. If others want to pin a date down, that's fine with me. I can always get my contributions merged into a future version!

@rvilarl
Copy link
Collaborator

rvilarl commented Oct 7, 2021

The date could be delayed but October sounds good. This would stop the feedback on 3.0.9 and setting a date would be a little bit more agile :)
@0xfe @mscuthbert @AaronDavidNewman @tommadams @sschmidTU @h-sug1no what do you think?

@mscuthbert
Copy link
Collaborator

I'm thumbs up. The only things that need to absolutely need to happen by 4.0.0 is anything that we've removed planning to add back in another way needs to be restored. I haven't seen anything like that left on the list. There's always 4.0.1 for fixing bugs we missed, 4.1 for adding more features, and 5.0 for doing more refactors. They can be November, December, and January releases if that's when we're ready. The numbers don't mean very much except for semantic versioning. Thanks!

@sschmidTU
Copy link
Contributor

sschmidTU commented Oct 8, 2021

I agree it should be beneficial to release a new version rather soon. There will probably be very quick feedback and new versions very soon after that - no matter how much time is taken for 4.0. So, instead of trying perfectionism, this seems more pragmatic to me :) it will also be nice to test the new npm release (i hope there will be one).
That being said - you all are doing an amazing job with 4.0, thank you very much and congratulations! I hope I can contribute more again soon as well, which might be kickstarted when i can start testing 4.0 with npm :)

@rvilarl
Copy link
Collaborator

rvilarl commented Oct 12, 2021

Now that we are changing the API in 4.0 I would like to add #1070 making those member protected and providing access functions

@ronyeh
Copy link
Collaborator Author

ronyeh commented Oct 12, 2021

I'm happy to try my best to support an October release if everyone else is into it. (Just be aware there are lots of little maintenance things that will have to be done, especially by Mohit....unless he wants one of us to do the npm publish for him 😨.)

I will be able to find some time starting later this week. I'd like to wrap up my Font PR and present it to you all to see if you'd like it in the 4.0 release.

@ronyeh
Copy link
Collaborator Author

ronyeh commented Oct 16, 2021

Question for y'all: what's the best way to do a staging release so that people can easily npm install and test against their projects?

@0xfe would it be okay for us to npm publish a "vexflow-beta" project that includes a big disclaimer about providing no support and to not use it in production? That way Simon and Myke and others can npm install from that project and tell us what things are broken, and where our changelog / migration guide is insufficient. (Yes, currently we don't have a migration guide, and the changelog is insufficient....)

For the vexflow-beta npm package, I would have the major and minor revision track with the release number. But the third digit will just be an integer that increments anytime we need to make another beta release, until we are ready to publish the official release. To prevent confusion, we could start the third number at like 9000 or something weird. Then just increment by +1 any time a PR is merged.

@ronyeh
Copy link
Collaborator Author

ronyeh commented Oct 21, 2021

Recently we had a discussion about whether we should be checking in releases (e.g., vexflow-min.js) into the repository.

From the main README, I see that we are serving releases from unpkg.com: https://github.com/0xfe/vexflow/#using-the-html-script-tag

If we want to eliminate the production builds from the repo, while continuing to support the unpkg URL, perhaps we can utilize .npmignore and .gitignore to keep the releases out of the repo, while maintaining the release builds in the npm package?

Another potential issue: If someone included a <script> tag that pointed to https://unpkg.com/vexflow/releases/vexflow-min.js.... will our 4.0 release automatically replace this URL? Currently this URL redirects to the 3.0.9 release, but after our release would unpkg automatically redirect to the 4.0 release and break deployed projects? In the future, we should update our readme to include the version number in the unpkg.com url. For example: https://unpkg.com/vexflow@3.0.9/releases/vexflow-min.js

@rvilarl
Copy link
Collaborator

rvilarl commented Oct 23, 2021

@0xfe when are you planning to do the October release?

@0xfe
Copy link
Owner

0xfe commented Oct 24, 2021

@ronyeh yes, the 4.0 release will break pages that use that URL -- I think it's fine though, because there's an expectation that things will break when you don't use version numbers in your tags.

We can either:

  • hardcode something like a -v4 to the filenames. E.g., ...releases/vexflow-v4-min.js
  • leave it as it is, and let people explicitly update their code (or their tags) if things break.

@0xfe
Copy link
Owner

0xfe commented Oct 24, 2021

@rvilarl I can push whenever we think we're ready. I'm waiting for you, Ron, Michael to say when.

I'll build/test, run some additional sanity tests, and push out a beta 4.0. I'd like to update vextab to support 4.0 before it becomes final.

@rvilarl
Copy link
Collaborator

rvilarl commented Oct 24, 2021

@rvilarl I can push whenever we think we're ready. I'm waiting for you, Ron, Michael to say when.

I'll build/test, run some additional sanity tests, and push out a beta 4.0. I'd like to update vextab to support 4.0 before it becomes final.

@0xfe I will stop submitting PRs. I believe that it is now more important to release. So once #1193 #1195 #1196 #1198 #1199 are either merge or rejected I am ready to go.
@ronyeh @mscuthbert what would you like to get done before release?

@AaronDavidNewman
Copy link
Collaborator

Hey guys, I have a small fix that I am pushing right now for some of the hardcoded metrics. It fixes issues:

  1. Petaluma grace notes look goofy
  2. Tremolo for Goneville and Petaluma font looks bad

I have no interest in whether it is included in 4.0.0 release, but I wanted to let you know. Since it is only metrics for these glyphs that are affected, there should be no impact on other visual regression tests.

@ronyeh
Copy link
Collaborator Author

ronyeh commented Oct 25, 2021

Hi all,

Sorry I've been holding up the release. I will devote more time this week to help wrap things up.

The Font PR is pretty close. I still need to add more tests. I'm happy with it, but unfortunately its scope has ballooned quite a bit. I'll share more details in the PR discussion over at: #1163

But the high level is that the Font PR contributes:

  • the .font property in all Elements are consistent, and follow CSS conventions.
  • the TextFont class has been broken up. The generic font code now resides in the Font class. The formatting code lives in the new TextFormatter class.
  • a new async API for setting the music engraving font, which is activated in the "lazy loaded font" scenario.

As for the release. Some questions I have are:

  • What goes into the final npm package? Previously it included the final release build and also all the *.js sources. (See vexflow on unpkg) Is that the same plan now that we are in TypeScript? Are there ever npm packages that are just the typescript files?
  • So when a project npm installs vexflow@4.0, how will they use it? Does the project have to be in TS to use vexflow@4.0? If a project is JS, do they just ignore the TS files and require or import the vexflow.js file?

@0xfe
Copy link
Owner

0xfe commented Oct 25, 2021

No problem, Ron. I'm in no rush, however this stuff can just keep going, so let's decide a way to time/feature box it. We can release with more frequency after 4.0.

I don't have very credible answers for your questions (outside of early thoughts) -- wondering if there are best practices here. @mscuthbert how do you publish your typescript npm packages?

@ronyeh
Copy link
Collaborator Author

ronyeh commented Oct 25, 2021

This short article helped me to understand a little more. https://dev.to/loonywizard/how-to-publish-typescript-package-to-npm-2k54

I assume we publish:

  • all the *.ts sources and tests like before
  • the production build *.js files under releases/4.0/. We can move old releases into releases/3.0.9/, etc.
  • maybe a declarations file??
    That way developers can choose to either use our typescript files directly, or require the full JS file in a JS only project. The only thing I'm worried about is whether vscode or other editors will get confused about which are the correct types, if there is both a declarations file and the original TS sources.

As for avoiding feature creep, my Font PR is my last one (though it's a big one...). I see a bunch of other PRs lining up behind mine though. Maybe we need to make an executive decision that any completely new PR submitted after today, October 25, is not included in 4.0 (unless it's super high priority enough that you personally approve it).

@ronyeh
Copy link
Collaborator Author

ronyeh commented Oct 25, 2021

Also, we all should first try "npm link" on a local clone of VexFlow to make sure our projects that depend on VexFlow still work. This can be a quick first pass sanity check before you publish the beta.

@ronyeh
Copy link
Collaborator Author

ronyeh commented Oct 25, 2021

OK, maybe we can (arbitrarily) declare that PR #1199 is the last new/independent PR that will make it into 4.0. :-)

Although my Font PR is a bit big, so upon your advice I might end up breaking it into several PRs for easier review.

@mscuthbert
Copy link
Collaborator

I don't have very credible answers for your questions (outside of early thoughts) -- wondering if there are best practices here. @mscuthbert how do you publish your typescript npm packages?

Oh, don't follow my practices -- I learned what I could about npm from Vexflow, so it's a circular argument. But I generally publish everything except generated docs, however, the only thing that people will really need are the min.js, the map file, and the .d.ts files that should be generated in types. I could not figure out how to create one single .d.ts file for music21j, so they're all separate files.

@mscuthbert
Copy link
Collaborator

I think that if the font PR is very big, why don't we put it in 4.1 or 4.0.0-beta-2 or something like that? I'd really like to do some testing on the 4.0 branch and Thanksgiving is my feature-freeze for infrastructure changes on another project; if I can't get a version of Vexflow working there by then, I'll need to wait until the summer to move up to 4.

@ronyeh
Copy link
Collaborator Author

ronyeh commented Oct 25, 2021

Hey @mscuthbert,

I'm pretty invested in getting this Font PR merged into 4.0, mostly because I'd like to take a short break from VexFlow afterward to spend time on my other projects.

I'll personally assist you with your project migration to 4.0. If I introduced a breaking change, I'll direct you to the new way of doing things. If the new way of doing things is terrible compared to 3.0.9, we can consider reverting some parts of the public API.

Mostly, I think it'll be good for me and Rodrigo to provide (a little) assistance with migrating your project & OSMD's project & other people's projects to 4.0, because it will help us uncover stumbling blocks. We can either log those issues in CHANGELOG and a migration guide, or if truly necessary we can add a 3.0.9 compatibility API or revert something.

@sschmidTU
Copy link
Contributor

sschmidTU commented Oct 28, 2021

I assume we publish:

  • all the *.ts sources and tests like before
  • the production build *.js files under releases/4.0/. We can move old releases into releases/3.0.9/, etc.
  • maybe a declarations file??

Sounds good. Javascript Devs will use the js build, Typescript devs will probably build from the .ts sources. (OSMD builds from the .js sources currently)
Though the 3.0.9 build shouldn't be included in the npm release for 4.0, @mscuthbert i'm guessing you meant that as well.

@ronyeh
Copy link
Collaborator Author

ronyeh commented Oct 28, 2021

I agree that old release builds shouldn't be in the npm package. They probably shouldn't be checked out by default in the main git repo either. However for maintainers like us, there is a benefit to having easy access to old release builds for regression testing purposes. Options:

  • Have releases in a vexflow-releases repo that we include as a submodule.
  • Add a script that checks out all the old releases from the git repo history via tags and commit IDs. This can be run once before regression testing begins.

@mscuthbert
Copy link
Collaborator

mscuthbert commented Oct 29, 2021 via email

@rvilarl
Copy link
Collaborator

rvilarl commented Oct 30, 2021

@AaronDavidNewman @mscuthbert @ronyeh @sschmidTU @0xfe I vote to make a first beta tomorrow and a second one next weekend. I am pretty sure that we will get many problems in the first one and we should just give it a try. Please use the thumbs. :)
The first migration PR was merged 17.03.

@ronyeh
Copy link
Collaborator Author

ronyeh commented Oct 30, 2021

Sure I'm fine with beta. But has anyone else tried npm link yet? I ran into issues, and that's why I submitted the build improvements PR.

@ronyeh
Copy link
Collaborator Author

ronyeh commented Oct 31, 2021

Hi all,

I want to ask a question about a possible slight restructuring of the npm package.

Proposal for the structure of the NPM Package

  • Keep the build artifacts in build/. When we ship to npm, the main file will be the minified build/vexflow.js.
  • Add releases/ to .npmignore. The npm package does not need to have previous releases.
  • Move the releases out of the main repo. The releases/ folder will start empty when you check out vexflow from github.
  • The releases can live in a separate GitHub repo (vexflow-releases, owned by Mohit). It can be added to vexflow as an optional git submodule. That way a VexFlow maintainer can quickly download all old releases for regression testing purposes. (A different approach is to write a script to pull old releases from commit hashes / git tags in the main repo. We would have to commit the npm release bundle, and then later delete it....)
  • Organize the releases/folder by version number.
    • releases/4.0.0/*
    • releases/3.0.9/*
    • releases/1.2.93/*

One possible side benefit of keeping the vexflow.js build inside the build/ folder, is that maybe we will be able to load flow.html directly from unpkg and have it show the correct test cases for that particular version. Currently, https://unpkg.com/vexflow@3.0.9/tests/flow.html is pointing to the ../build/vexflow-tests.js which doesn't exist in the npm package. It would be cool if that page just worked :-).

What does everyone think? I'd be happy to let someone else take the lead on the restructuring of the npm package (e.g., @rvilarl). I have my hands full already trying to get my main Font PR reviewed and merged, hehe.

This idea was inspired a little by how Tone.js structures their npm release. (I'm a fan of Tone.js if you can't tell.)
See their npm release at:

@ronyeh
Copy link
Collaborator Author

ronyeh commented Nov 8, 2021

Check out the latest version here:

https://github.com/ronyeh/vexflow/tree/4.0.0-alpha

The accompanying typescript example is here:

https://github.com/ronyeh/vexflow-example-typescript

You can also take a look at the tsconfig.json in the typescript example project for a more direct way to use the vexflow typescript source files directly.

Have fun, and let me know if you have any questions.

I'll put together some docs / tutorials soon on how to integrate with VexFlow 4. I successfully did it with VexTab already, and there actually weren't many breaking changes.

@ronyeh ronyeh pinned this issue Nov 14, 2021
@ronyeh
Copy link
Collaborator Author

ronyeh commented Nov 14, 2021

Congrats everyone on getting to 4.0-beta!

I will keep this issue open as a meta / tracker issue and we can close it once the official release is out.

Suggested PRs for refinements to 4.0 beta 2

Better Documentation

I'd suggest updating and migrating the wiki docs into markdown files stored in the vexflow/docs/ folder. The current contents can go into docs/3.0.9/ for now. The wiki can remain as bullet lists to things like JSFiddle examples, and direct links to the *.md files in the docs/ folder. These should be rendered automatically when browsing through GitHub. As a tiny example, our changelog in the wiki sidebar already links directly into the repo's CHANGELOG file. See: https://github.com/0xfe/vexflow/wiki

It's still nice to have really short (hello world / getting started) examples directly in the wiki and initial README. However, anything longer is tough to maintain / version / compile if it's not in the repository. For example, I was updating Rodrigo's docs on fonts from this summer, and I encountered a bunch of copy & paste typos. Totally understandable, since it's tough to write code inside triple tick marks inside the GitHub wiki editor!

Native ESM support

This is coming soon, and I will request your help in testing it for beta 2.

Remove releases/ folder

This is coming shortly. The npm release is now directly stored in vexflow/build/. They can be served from jsdelivr or unpkg from the build/ folder, as many other libraries do. As an aside, I have been doing some testing this week and unpkg seemed less responsive to me than jsdelivr. Perhaps someone can do research into this? @0xfe I'd at least suggest including the jsdelivr CDN as an equal alternative to unpkg, so folks know that it is an alternative.

Any more suggested refinement PRs, for beta 2?

If you run into roadblocks during your migration to VexFlow 4 beta, please file a separate issue and @ tag me.

@rvilarl
Copy link
Collaborator

rvilarl commented Nov 22, 2021

My suggestion is to reduce the number of open issues to 50.

@ronyeh
Copy link
Collaborator Author

ronyeh commented Nov 22, 2021

I don't have an problem with keeping old issues around if we (eventually) want to handle them. However, if there is no momentum for something, or if an issue is no longer actionable due to it being outdated or too vague, we can always close it. The originator of the bug can always submit a new one if it still applies to his/her project, and we can investigate (as long as we can repro).

@0xfe
Copy link
Owner

0xfe commented Nov 28, 2021

+1 for just closing old issues

@rvilarl rvilarl mentioned this issue Jan 1, 2022
@ronyeh
Copy link
Collaborator Author

ronyeh commented Jan 1, 2022

Moving the 4.0.x release discussion to:
#1281

@ronyeh ronyeh closed this as completed Jan 1, 2022
@ronyeh ronyeh unpinned this issue Jan 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants