search

Ircam-RnD / binauralFIR

Binaural module for the Web Audio API
https://ircam-rnd.github.io/binauralFIR/
BSD 3-Clause "New" or "Revised" License
145 stars 16 forks source link

build: refactor build pipeline #5

Closed MylesBorins closed 8 years ago

MylesBorins commented 8 years ago

This PR removes the need for the custom build scripts living in ./bin

AFAIK this is a feature complete replacement except for one change. The input point of the app has been modified to lib/binaural-fir.js and the babelify transform has been added (and included in the package.json).

If someone is requiring this package in a browserify workflow the transform will automagically be applied to the subtree when required.

I'm not 100% you will like all of these changes but figured it is a good place to start.

The script npm run build will now build all the assets.

b-ma commented 8 years ago

Hey @TheAlphaNerd,

Many thanks for your proposal, we have checked your PR and the many interesting things inside. However, there is aspects of it that we have to discuss and test before accepting the PR, and we will be able to do that only in few days.

Our main concern with the pipeline you are proposing (as we use this script in other projects, some of which are way bigger than this one) is the build time when using the package in the final application (this aspect is why the script has been made this way - splitting the babel and browserify transformations - in the first time). This is something that we really need to test in a bigger module than this one.

We will review that as soon as possible. Thanks !

MylesBorins commented 8 years ago

Thanks for taking the time to check.

May is suggest you check both initial and subsequential builds in your benchmarking.

Watchify does fairly aggressive caching, and while initial build may be slower in a large project, subsequential builds (while watchify is running) may be substantially faster.

MylesBorins commented 8 years ago

Since this has been open for so long I'm going to close and assume ya'll are going to pass on merging

jipodine commented 8 years ago

Thank you for the proposal. I tried to put everything as npm scripts (see branch next), as I like this simple solution. However, there 2 main problems: