Skip to content

Conversation

@mstade
Copy link
Member

@mstade mstade commented Nov 4, 2015

@grancalavera this oughta do it. When we merge this, we should also merge this: websdk/nap-examples#1.

If we had more dependencies, we'd need to have a better story for
this, but it'll do for now.
@mstade
Copy link
Member Author

mstade commented Nov 4, 2015

sigh... Of course the build fails because npm..

@mstade
Copy link
Member Author

mstade commented Nov 4, 2015

Heh, actuallly this is because in the makefile we set the path to the local node_modules/.bin folder. Node 5 ships with NPM 3, which dedupes and installs modules in an as-flat-as-possible manner, meaning somewhere deep in the dev dependencies there's something that depends on the touch module. This then understandably gets confused because we touch the node_modules dir after npm install, and that module doesn't really deal well with directories. (See: isaacs/node-touch/issues/8)

So yeah – the build fails because NPM, but I guess it's also pretty stupid to prepend $PATH instead of appending. Will fix.

This fixes the broken build when npm 3 is used.
@mstade
Copy link
Member Author

mstade commented Nov 4, 2015

That's strange, the build still fails with the same error – but it works fine locally with Node 5 and npm 3. Investigating..

We may just want to look into Wercker instead, so we can have a
deterministic environment for the builds.
@mstade
Copy link
Member Author

mstade commented Nov 5, 2015

Seems travis-ci/travis-ci#4862 covers what's going on, and offers a workaround. I'm going to implement the workaround for now, and then look into using wercker instead at some point down the line.

@mstade mstade changed the title Quick and dirty change to exclude rhumb from the build Exclude rhumb from the build Nov 5, 2015
mstade added a commit that referenced this pull request Nov 5, 2015
Exclude rhumb from the build
@mstade mstade merged commit c60e5ec into master Nov 5, 2015
@mstade mstade deleted the exclude-rhumb branch November 5, 2015 13:10

Choose a reason for hiding this comment

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

Are you bumping the version by hand?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, using npm version. I'm going with the no-one-really-cares-about-semver-before-1.0 approach, so bumping minor for the breaking changes and patch for non-breaking. Once we hit 1.0.0 though we should take better care.

Choose a reason for hiding this comment

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

cool

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