Configure Circle CI #20

djbe · 2017-02-15T18:48:47Z

Note: Once this is merged, don't forget to:

Adapt the GitHub settings of this repo to make circle the mandatory status check instead of travis

AliSoftware · 2017-02-19T16:51:25Z

We should create the Rakefle for each repo before starting to adjust the circle.yml files: it would be more readable for circle.yml to invoke rake test than the whole xcodebuild command

djbe · 2017-02-19T17:26:28Z

Sure, but we should add versions depending on environment. If CI, output logs to $CIRCLE_ARTIFACTS and $CIRCLE_TEST_REPORTS. Also each rake task should be separate, so we can eventually parallelize stuff.

AliSoftware · 2017-02-19T22:17:03Z

Rakefile

- sh "swift test"
- xcpretty "xcodebuild -workspace StencilSwiftKit.xcworkspace -scheme Tests test-without-building"
+desc 'Run SPM Unit Tests'
+task :spm_test => :spm_build do


Tip: you can use take namespaces to group relative tasks together.

A namespace 'spm' do … end and a namespace 'xcodebuild' do … end would be nicer to read, and then you can name your tasks just build and test and refer to them as rake spm:test etc.

AliSoftware

👌

djbe · 2017-02-19T23:00:29Z

Only missing thing is a review from Danielle and we're good to go.

Edit: well that, and switch the github configuration to require circle builds instead of travis.

AliSoftware

TODO: Improve CP setup caching

AliSoftware · 2017-02-19T23:02:44Z

circle.yml

+ - gem install xcpretty --no-rdoc --no-ri --no-document --quiet
+ - if [[ $CIRCLE_BRANCH == master ]]; then pod setup; fi
+ cache_directories:
+ - "~/.cocoapods/repos/master"


@djbe Well I think we still need to change that pod setup + cache_directories strategy to use what @dantoml told us about in Slack, to improve that part before consider merging.

That's what I meant with input from @dantoml 😄

AliSoftware · 2017-02-19T23:11:30Z

Rakefile

+
+ desc 'Run SPM Unit Tests'
+ task :test => :build do
+ plain("swift test", "spm_build")


Shouldn't the name param be spm_test there instead of spm_build (especially to avoid the risk of overriding the log output from build)?

I tried using a ruby trick to get the calling method's name, but keep getting block (2 levels) in <top (required)>, so gave up on that.

You could do that by capturing the 1st argument passed to the block, which is the rake task object itself, and has a .name property:

namespace :foo do task :bar do |task| puts "Name: #{task.name}" // task.name is indeed "foo:bar" end end

AliSoftware · 2017-02-23T23:30:14Z

Rakefile

+ desc 'Lint the code'
+ task :code do |task|
+ plain("PROJECT_DIR=. ./Scripts/swiftlint-code.sh", task)
+ end


Just a thought: I'm not sure about the readability/logic of grouping those two commands, still undecided.

Because "linting a pod" and "linting the code" seems two different kinds of lints to me. One is checking that the podspec "compiles" (the podspec file doesn't have any syntax error, and that a project with that pod would compile), the other is checking for code style issues.

Maybe it would be more logical to put everything related to SwiftLint in the Rakefile anyway (I mean instead of having separate .sh scripts to install swiftlint and lint the code — which were there at first because we didn't have any Rakefile or because we found this install_swiftlint script as is) and have rake swiftlint:install, rake swiftlint:sourcecode and rake swiftlint:tests tasks? (and then concerning the pod lib lint, not sure it's worth having a dedicated rake task?)

AliSoftware · 2017-02-23T23:32:19Z

StencilSwiftKit.xcworkspace/xcshareddata/StencilSwiftKit.xcscmblueprint

@@ -0,0 +1,30 @@
+{
+ "DVTSourceControlWorkspaceBlueprintPrimaryRemoteRepositoryKey" : "804E0A642F9B0288E24EC6843BD9BD71D8BD7EA7",


I never know if this file should be pushed on git, I see it being ignored by a lot of .gitignored on OSS projects and I generally gitignore it too, I think because otherwise it gets updated almost on every commit… I believe it's only useful for Xcode to have metadata about the repo, but who uses Xcode's integrated SCM tool anyway

Huh, I thought I deleted that. Must be in another branch

AliSoftware · 2017-02-23T23:34:19Z

Tests/StencilSwiftKitTests/CallNodeTests.swift

- 
- func testRender() {
+
+ func testRender() throws {


I didn't even know XCTest would still recognize functions as tests when they are marked with throws, I thought only func test<Whatever>() functions with this exact signature type would be recognized! (wait, that test actually does get executed, right?)

I saw it as a little know trick recently related to XCTest, maybe one of Natasha's newsletters or something? Can't remember.

Anyway, the test still gets executed, during testing it just has a different name (AndReturnError: gets appended): [Tests.CallNodeTests testRenderAndReturnError:]

AliSoftware · 2017-02-23T23:37:38Z

Tests/StencilSwiftKitTests/TestsHelper.swift

+ "\(lhsNum)\n" +
+ "======\n" +
+ "\(rhsNum)\n" +
+ "<<<<<< expected"


I think an array of strings that gets join'd is more efficient that successive string concatenation… and compiles faster (because + is so overloaded) too

AliSoftware · 2017-02-23T23:38:42Z

circle.yml

@@ -5,13 +5,15 @@ machine:
 dependencies:
 pre:
 - gem install xcpretty --no-rdoc --no-ri --no-document --quiet
+ - Scripts/install_swiftlint.sh
 - if [[ $CIRCLE_BRANCH == master ]]; then pod setup; fi


This needs to be converted to the "download_from_s3" trick

Jup, added it while you were typing 😆

AliSoftware · 2017-02-23T23:44:39Z

circle.yml

 - rake xcode:test
 - rake spm:test
 post:
- - if [[ $CIRCLE_BRANCH == master ]]; then rake lint; fi
+ - if [[ $CIRCLE_BRANCH == master ]]; then rake lint:pod; fi


Other alternative: [[ $CIRCLE_BRANCH == master ]] && rake lint:pod (which would actually be [[ $CIRCLE_BRANCH == master ]] && pod lib lint --quick if we take the pod lint out of the Rakefile).
I have no real preference, just saying in case you prefer && over if; then; fi

AliSoftware · 2017-02-24T11:43:41Z

Tests/StencilSwiftKitTests/TestsHelper.swift

@@ -42,7 +45,14 @@ func diff(_ result: String, _ expected: String) -> String? {
 }
 let lhsNum = addLineNumbers(slice(lhsLines, 4)).joined(separator: "\n")
 let rhsNum = addLineNumbers(slice(rhsLines, 4)).joined(separator: "\n")
- return "\(msgColor)Mismatch at line \(badLineIdx)\(reset)\n>>>>>> result\n\(lhsNum)\n======\n\(rhsNum)\n<<<<<< expected"
+ return [
+ "\(msgColor)Mismatch at line \(badLineIdx)\(reset)\n",


Extra \n (which should be added by the join: is it intentional (so that it ends up with two \n for this line and thus a blank line in the result) or not?

No, not intentional ☹️

djbe · 2017-02-24T11:55:24Z

How do we merge this PR? The github configuration'll need to be changed to require circle tests instead of travis before we can merge it.

AliSoftware · 2017-02-24T11:59:27Z

Done :)

djbe force-pushed the feature/circleci branch 3 times, most recently from 2ac219c to dff7b4b Compare February 15, 2017 23:22

djbe added the type: internal label Feb 16, 2017

djbe added 4 commits February 19, 2017 03:26

test

2a9da2d

remove travis

a42fade

try caching cocoapods

d2f5c1e

store output to designated circle paths

f0939f9

AliSoftware force-pushed the feature/circleci branch from dff7b4b to f0939f9 Compare February 19, 2017 02:32

rewrite rakefile to output circle artefacts on CI

4ee70bf

AliSoftware reviewed Feb 19, 2017

View reviewed changes

namespace commands

15c5f60

djbe force-pushed the feature/circleci branch from 782c800 to 15c5f60 Compare February 19, 2017 22:46

switch status image from travis to circle

18262e8

AliSoftware reviewed Feb 19, 2017

View reviewed changes

AliSoftware requested changes Feb 19, 2017

View reviewed changes

AliSoftware reviewed Feb 19, 2017

View reviewed changes

small fix

703c749

djbe force-pushed the feature/circleci branch from 08a440b to 703c749 Compare February 19, 2017 23:32

get name from task

011f3a3

djbe force-pushed the feature/circleci branch from 1317734 to 011f3a3 Compare February 19, 2017 23:45

This was referenced Feb 19, 2017

Configure CircleCI SwiftGen/SwiftGenKit#10

Merged

Configure CircleCI SwiftGen/templates#24

Merged

Configure CircleCI SwiftGen/SwiftGen#269

Merged

djbe added 2 commits February 23, 2017 18:16

code linting

e124d1e

fix swift lint issues

10fa112

djbe force-pushed the feature/circleci branch from 278fb85 to d25548d Compare February 23, 2017 22:12

use pod specs from S3

68c23f0

djbe force-pushed the feature/circleci branch from d25548d to 68c23f0 Compare February 23, 2017 22:28

AliSoftware reviewed Feb 23, 2017

View reviewed changes

djbe added 3 commits February 24, 2017 11:26

use join instead of +

1539a66

move swiftlint scripts into rakefile

283e7e3

delete and ignore useless xcode file

10d9a3e

djbe force-pushed the feature/circleci branch from db5de2c to 8837c49 Compare February 24, 2017 10:54

AliSoftware reviewed Feb 24, 2017

View reviewed changes

AliSoftware approved these changes Feb 24, 2017

View reviewed changes

forgot to update circle.yml

97e78a3

djbe force-pushed the feature/circleci branch from 8837c49 to 97e78a3 Compare February 24, 2017 11:45

AliSoftware merged commit a336166 into master Feb 24, 2017

AliSoftware deleted the feature/circleci branch February 24, 2017 12:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Configure Circle CI #20

Configure Circle CI #20

djbe commented Feb 15, 2017 •

edited by AliSoftware

Loading

AliSoftware commented Feb 19, 2017 •

edited

Loading

djbe commented Feb 19, 2017

AliSoftware Feb 19, 2017

AliSoftware left a comment

djbe commented Feb 19, 2017 •

edited

Loading

AliSoftware left a comment

AliSoftware Feb 19, 2017

djbe Feb 19, 2017

AliSoftware Feb 19, 2017

djbe Feb 19, 2017

AliSoftware Feb 19, 2017 •

edited

Loading

AliSoftware Feb 23, 2017

AliSoftware Feb 23, 2017

djbe Feb 24, 2017

AliSoftware Feb 23, 2017

djbe Feb 24, 2017

AliSoftware Feb 24, 2017

AliSoftware Feb 23, 2017

AliSoftware Feb 23, 2017

djbe Feb 24, 2017

AliSoftware Feb 23, 2017 •

edited

Loading

AliSoftware Feb 24, 2017

djbe Feb 24, 2017

djbe commented Feb 24, 2017

AliSoftware commented Feb 24, 2017

		@@ -0,0 +1,30 @@
		{
		"DVTSourceControlWorkspaceBlueprintPrimaryRemoteRepositoryKey" : "804E0A642F9B0288E24EC6843BD9BD71D8BD7EA7",

Configure Circle CI #20

Configure Circle CI #20

Conversation

djbe commented Feb 15, 2017 • edited by AliSoftware Loading

AliSoftware commented Feb 19, 2017 • edited Loading

djbe commented Feb 19, 2017

Choose a reason for hiding this comment

AliSoftware left a comment

Choose a reason for hiding this comment

djbe commented Feb 19, 2017 • edited Loading

AliSoftware left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AliSoftware Feb 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AliSoftware Feb 23, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

djbe commented Feb 24, 2017

AliSoftware commented Feb 24, 2017

djbe commented Feb 15, 2017 •

edited by AliSoftware

Loading

AliSoftware commented Feb 19, 2017 •

edited

Loading

djbe commented Feb 19, 2017 •

edited

Loading

AliSoftware Feb 19, 2017 •

edited

Loading

AliSoftware Feb 23, 2017 •

edited

Loading