-
Notifications
You must be signed in to change notification settings - Fork 146
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
chore: fix TravisCI build #46
chore: fix TravisCI build #46
Conversation
8c087ca
to
a4ae2a2
Compare
- "0.10" | ||
|
||
before_install: | ||
- '[ "${TRAVIS_NODE_VERSION}" != "0.8" ] || npm install -g npm@1.4.28' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this line is unnecessary! Otherwise LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aymericbeaumet Ha! I must say that I've copied this line as a monkey, without fully understanding it :-/ Shame on me. The thing is that without this line it wasn't working:
https://travis-ci.org/karma-runner/karma-ng-html2js-preprocessor/jobs/37902879
Do you know what this line supposed to do / why it is here (Pawel looks up bash syntax feeling ashamed...).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It installs a specific npm version in the case node 0.8
is detected. It's weird because I thought updating npm to the latest version would suffice.
It seems npm 1.4.28
is the latest version supported by node. So let's just leave it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guys this line fixes problem with updating npm
on node@0.8 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@maksimr thnx, although I still don't understand why do we need to call both npm install -g npm@1.4.28
and npm install -g npm@latest
on node 0.8. Shouldn't this line
'[ "${TRAVIS_NODE_VERSION}" != "0.8" ] || npm install -g npm@1.4.28'
be enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pkozlowski-opensource seems npm install -g npm@1.4.28
should be enough to fix all problems associated with npm
and node@0.8
.
Because we ran npm install -g npm
for the same reasons(fix problem with node and travis).
Also latest version of npm
is 2.*
that may not solve the problem with node@0.8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's just stick with npm install -g npm@1.4.28
for now.
a4ae2a2
to
217292e
Compare
Fixes #45