Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
andnpm 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 withnpm
andnode@0.8
.Because we ran
npm install -g npm
for the same reasons(fix problem with node and travis).Also latest version of
npm
is2.*
that may not solve the problem with node@0.8There 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.