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

fetching latest p5.js code from npm #501

Merged
merged 2 commits into from
Jul 16, 2020

Conversation

endurance21
Copy link
Collaborator

@endurance21 endurance21 commented Jul 8, 2020

#484

some highlights

  • installing latest p5 from npm version "1.0.0"

  • post-install Scripts :

    • copying p5.js file from /node_modules to /lib folder .

Copy link
Member

@therewasaguy therewasaguy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this, @endurance21 !

I'm wondering if we should re-add p5.js (and p5.min.js) to the git repo as part of this PR? They were removed in #483.

I'd prefer to figure out how to avoid checking those files into the repo and to just let npm install those files when we need them:

  • for local development (which this PR handles 🎉 )
  • for automated testing (I believe we're also covered by the post-install script here?)
  • and for gh-pages (this one I'm not sure about)

We could look at gh-pages as a separate ticket if you'd prefer to focus on the install script with this PR. We could use a library like this https://www.npmjs.com/package/gh-pages to publish just what we need to the gh-pages branch, without checking it into the main branch. For this PR, I'd say either add both files (p5.js and p5.min.js), or don't add either of them.

lib/p5.sound.js Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
Gruntfile.js Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
Copy link
Collaborator

@LilaShiba LilaShiba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we nixing this for a smaller pr?

 * modifications in postinstall script
 * removed p5.js file
@endurance21
Copy link
Collaborator Author

@therewasaguy @kyle1james it's good to go now !

src/audioWorklet/index.js Show resolved Hide resolved
@endurance21 endurance21 merged commit d30c43d into processing:master Jul 16, 2020
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.

3 participants