djipco / webmidi

Tame the Web MIDI API. Send and receive MIDI messages with ease. Control instruments with user-friendly functions (playNote, sendPitchBend, etc.). React to MIDI input with simple event listeners (noteon, pitchbend, controlchange, etc.).
Apache License 2.0
1.53k stars 115 forks source link

Proposal: .gitignore dist/ #253

Closed jjeff closed 2 years ago

jjeff commented 2 years ago

Having the dist/ directory in Git makes pull requests unnecessarily confusing and unwieldy.

I'm guessing that 98% of users are using WebMidi from the npm package. I propose:

  1. Remove the dist/ directory from the Git repository - as the first commit
  2. Then add dist/ to the .gitignore file - as the second commit

Then you'd have a workflow where the dist/ directory gets built locally before testing and/or publishing to npm. But there wouldn't be a lot of diff craziness in pull requests or commits.

djipco commented 2 years ago

Yes, I agree.

The only drawback is for those who just want to grab the IIFE version and run with it. I guess that, in such a case, they could simply retrieve webmidi.iife.js from JSDelivr.

We should adapt the Installing manually section of the documentation to reflect the change.

jjeff commented 2 years ago

I don't have much experience with this, but I think you could do process with GitHub Releases similar to npm publish. So there would be a .zip containing the npm-like content that people could download from GitHub. I'll bet there's a CLI tool for this somewhere.

Vexcited commented 2 years ago

Hello, I want to help for this issue !

I've already forked the project and made some modifications.

Now for the releases (to use manually with .zip), I think we could rewrite current script in scripts/release/release.js to, instead of commit the dist folder, release the dist folder to GitHub Releases.

djipco commented 2 years ago

Your help is very much appreciated! Merci!

If we are going to rework this, we probably should look at Release It!. It simplifies several of the necessary steps involved when releasing a new version. It would, amongst other things, allow us to create a GitHub tagged release which means the instructions in the Installing Manually section of the documentation would stay the same.

Even if it's an interactive tool, it can still be used programmatically.

Vexcited commented 2 years ago

Even if it's an interactive tool, it can still be used programmatically.

From what I've seen, I don't think we should do something programmatic anymore if we're using this tool because we can define a release-it configuration directly in our package.json and then call their release-it command in the NPM scripts.

Also, we need a GitHub token to programmatically release to GitHub Releases, so I think we should use the interactive way in the NPM scripts to override this (and use the browser release way as it says in the GitHub Releases part of the library)

Let me know what do you think about it !

djipco commented 2 years ago

From what I've seen, I don't think we should do something programmatic anymore if we're using this tool because we can define a release-it configuration directly in our package.json and then call their release-it command in the NPM scripts.

If everything can be configured in package.json, that's the way to go.

Also, we need a GitHub token to programmatically release to GitHub Releases, so I think we should use the interactive way in the NPM scripts to override this

Setting a token isn't hard to do but I guess we should experiment to see what works best.

Vexcited commented 2 years ago

Okay, I'll try to implement that.

For the token, it's just about setting GITHUB_TOKEN in environment when the script is running.

More informations about it here, says that we can use dotenv and a .env file. Guess that would be the easiest and fastest way to implement the feature.

djipco commented 2 years ago

Guess that would be the easiest and fastest way to implement the feature.

Yes, exactly!

djipco commented 2 years ago

Thanks to @Vexcited and his work on PR https://github.com/djipco/webmidi/pull/254, this has been implemented. I had to create a few releases to nail down all the details but it seems stable now.

P.S. Users who want to download the library manually can still go to the latest release page.