Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

chore(jenkins): run Jenkins builds on Node 4 (via nvm) #13568

Closed
wants to merge 1 commit into from
+13 −8
Closed

chore(jenkins): run Jenkins builds on Node 4 (via nvm) #13568

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions scripts/jenkins/build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ echo "#################################"
# Enable tracing and exit on first failure
set -xe

scripts/jenkins/set-node-version.sh

# This is the default set of browsers to use on the CI server unless overridden via env variable
if [[ -z "$BROWSERS" ]]
then
Expand All @@ -19,6 +21,7 @@ rm -f angular.js.size


# BUILD #
npm install -g grunt-cli
npm install --color false
grunt ci-checks package --no-color

Expand Down
9 changes: 1 addition & 8 deletions scripts/jenkins/master.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,7 @@ function init {

function build {
cd ../..

if [[ $NO_TEST == "true" ]]; then
npm install --color false
grunt ci-checks package --no-color
else
scripts/jenkins/build.sh
fi

scripts/jenkins/build.sh
cd $SCRIPT_DIR
Copy link
Member

Choose a reason for hiding this comment

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

If the --no-test option is not used any more, should we remove it from ARG_DEFS above ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yes. Good spot

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added at 4bcb307 (and similar commits in each of the other branches).

}

Expand Down
2 changes: 2 additions & 0 deletions scripts/jenkins/release.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,10 @@ function init {
}

function build {
./set-node-version.sh
cd ../..

npm install -g grunt-cli
npm install --color false
grunt ci-checks package --no-color

Expand Down
7 changes: 7 additions & 0 deletions scripts/jenkins/set-node-version.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#!/bin/bash

# Install nvm for this shell
source ~/.nvm/nvm.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

Does jenkins provide this (i.e. where does it come from)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is manually installed on the server.

Copy link
Member

Choose a reason for hiding this comment

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

Could we print the nvm/Node/npm versions used? Could be useful for debugging.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that it does do that automatically...

+ scripts/jenkins/set-node-version.sh
v4.2.3 is already installed.
Now using node v4.2.3 (npm v2.14.7)


# Use node.js at 4.2.x
nvm install 4.2
Copy link
Member

Choose a reason for hiding this comment

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

Is nvm install ... sufficient or do we also need nvm use ... ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nvm install is sufficient.

Copy link
Member

Choose a reason for hiding this comment

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

4 would be enough, the latest 4.x.y release is an LTS, it so happens it's 4.2.y in fact.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From the node.js docs:

Minors: Minors are the addition and refinements of APIs or subsystems. They do not generally change APIs or introduce backwards compatible breaking changes, except where unavoidable. These are mostly completely additive releases.

I think it is safer to stick with a fixed minor release for automatic updates

Copy link
Member

Choose a reason for hiding this comment

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

OK. It doesn't matter here anyway as there will be no Node.js 4.3.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see: https://github.com/nodejs/LTS/