Skip to content
This repository has been archived by the owner. It is now read-only.

Jenkinsfile for CI #13

Merged
merged 1 commit into from Mar 27, 2018
Merged

Jenkinsfile for CI #13

merged 1 commit into from Mar 27, 2018

Conversation

@sharath-cliqz
Copy link
Contributor

@sharath-cliqz sharath-cliqz commented Mar 22, 2018

Continuous Integration using Jenkins.

  • Added a Jenkinsfile to setup a build environment and build the APP for an iOS Simulator.
Copy link
Member

@chrmod chrmod left a comment

overall really nice. my comments aim to improve quality and simplify the setup

Jenkinsfile Outdated
set -x
xcodebuild -version
pkgutil --pkg-info=com.apple.pkg.CLTools_Executables
sudo xcodebuild -license accept

This comment has been minimized.

@chrmod

chrmod Mar 23, 2018
Member

why we have to do it on job level? the vm image should already have it covered

This comment has been minimized.

@sharath-cliqz

sharath-cliqz Mar 26, 2018
Author Contributor

fixed it. Has been done on the Image level now.

Jenkinsfile Outdated
sh '''#!/bin/bash -l
set -e
set -x
xcodebuild -version

This comment has been minimized.

@chrmod

chrmod Mar 23, 2018
Member

as we specify image explicitly config.vm.box = "ios-xcode9.2" this is not very useful

Jenkinsfile Outdated
pkgutil --pkg-info=com.apple.pkg.CLTools_Executables
sudo xcodebuild -license accept
brew update
brew list carthage &>/dev/null || brew install carthage

This comment has been minimized.

@chrmod

chrmod Mar 23, 2018
Member

why we need he list?

This comment has been minimized.

@sharath-cliqz

sharath-cliqz Mar 26, 2018
Author Contributor

Just checking if it is already installed. Maybe will have to look for a better way.

Jenkinsfile Outdated
stage('Build') {
timeout(10) {
sh '''#!/bin/bash -l
set -x

This comment has been minimized.

@chrmod

chrmod Mar 23, 2018
Member

is -x doing anything here?

This comment has been minimized.

@sharath-cliqz

sharath-cliqz Mar 26, 2018
Author Contributor

removed.

'''
}
stage('Build') {
timeout(10) {

This comment has been minimized.

@chrmod

chrmod Mar 23, 2018
Member

10 may be not enough.. ?

This comment has been minimized.

@sharath-cliqz

sharath-cliqz Mar 26, 2018
Author Contributor

It usually builds in 3-4Mins (Max) So i thought 10 was a reasonable number.

Jenkinsfile Outdated
rm -rf Cartfile.resolved
chmod a+x bootstrap.sh
./bootstrap.sh
yarn install

This comment has been minimized.

@chrmod

chrmod Mar 23, 2018
Member

btw. new npm have CI mode, you use it by npm ci - they say it is faster and more strict than regular npm install. maybe we can use it instead of yarn

Jenkinsfile Outdated
set -e
set -x
xcodebuild -version
pkgutil --pkg-info=com.apple.pkg.CLTools_Executables

This comment has been minimized.

@chrmod

chrmod Mar 23, 2018
Member

what does it provide?

This comment has been minimized.

@sharath-cliqz

sharath-cliqz Mar 26, 2018
Author Contributor

removed.

Jenkinsfile Outdated
node(nodeId) {
stage("Checkout") {
checkout scm
}

This comment has been minimized.

@chrmod

chrmod Mar 23, 2018
Member

please fix indent

This comment has been minimized.

@sharath-cliqz

sharath-cliqz Mar 26, 2018
Author Contributor

Fixed.

This comment has been minimized.

@chrmod

chrmod Mar 26, 2018
Member

does not look like :-)

@Library('cliqz-shared-library@vagrant') _

node('mac-mini-ios') {
writeFile file: 'Vagrantfile', text: '''

This comment has been minimized.

@chrmod

chrmod Mar 23, 2018
Member

lets put that on shared library with some configuration options

This comment has been minimized.

@chrmod

chrmod Mar 26, 2018
Member

as the contents of vagrant file is not parameterized, it can be put on the variable in the top of the file, it should improve readability

Jenkinsfile Outdated
vagrant.inside(
'Vagrantfile',
'/jenkins',
4, // CPU

This comment has been minimized.

@chrmod

chrmod Mar 23, 2018
Member

4 cpu is quite a lot :D have you benchmarked how number of cpu affect build time?

This comment has been minimized.

@sharath-cliqz

sharath-cliqz Mar 26, 2018
Author Contributor

@chrmod When we are running tests on the Simulator, it is better to use 4 CPUs to make it finish faster.
As of now reduced them to 2 and we can benchmark them at a later point of time when the tests are ready.

@sharath-cliqz sharath-cliqz force-pushed the sharath-cliqz:testci branch from a40ff74 to 01c8f70 Mar 26, 2018
@sharath-cliqz sharath-cliqz force-pushed the sharath-cliqz:testci branch from 01c8f70 to 60086d7 Mar 26, 2018
@chrmod
chrmod approved these changes Mar 26, 2018
Copy link
Member

@chrmod chrmod left a comment

looks really good

@Library('cliqz-shared-library@vagrant') _

node('mac-mini-ios') {
writeFile file: 'Vagrantfile', text: '''

This comment has been minimized.

@chrmod

chrmod Mar 26, 2018
Member

as the contents of vagrant file is not parameterized, it can be put on the variable in the top of the file, it should improve readability

timeout(10) {
sh '''#!/bin/bash -l
set -e
xcodebuild -workspace Client.xcworkspace -scheme "Fennec" -sdk iphonesimulator -destination "platform=iOS Simulator,OS=11.2,id=185B34BB-DCB8-4A17-BDCA-843086B67193" ONLY_ACTIVE_ARCH=NO -derivedDataPath clean build

This comment has been minimized.

@chrmod

chrmod Mar 26, 2018
Member

would be nice to break this long line

finally {
stage('Cleanup') {
sh '''#!/bin/bash -l
xcrun simctl uninstall booted com.cliqz.ios.newCliqz || true

This comment has been minimized.

@chrmod

chrmod Mar 26, 2018
Member

is this line printed in jenkins log? I'm asking as there is no set -x

@timoteipalade timoteipalade merged commit b8145f1 into ghostery:master Mar 27, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants