Configure CircleCI

[djbe]

Fixes #20, keep track of SwiftGen/StencilSwiftKit#20 for any changes that need to be copied here.

[djbe]

@AliSoftware To get faster builds, we could only compile the output files if they've changed, and only those that have changed. Or would that be too much of an optimisation?

[AliSoftware]

That's a nice idea. How do you propose to determine that in a deterministic way?
Using octokit to query GitHub for the PR for the list of changes files? (Would require a GH token but we might use the one we use for Danger)

[djbe]

In the Danger PR, you do something like:

if !git.modified_files.include?("CHANGELOG.yml")

I was hoping to do something similar, but just matching the path to the Expected folder.

[AliSoftware]

Yeah but we can do that because Danger is driving the Dangerfile, and what it does for that is using octokit (the gem to make GH API requests in ruby easier) to get the PR metadata and exposing it to the Dangerfile

So if we want to do what you're suggesting we must either replicate this (using octokit in our Rakefile to fetch the PR metadata including the list of changed files) or embed the step of compiling the generated files in Expected/… inside the Dangerfile as a Danger step rather than as a CI step of its own like the others

[djbe]

Right, right. Any preference for this? Or do we consider this as something nice to have, but maybe later? 😄

[AliSoftware]

If we're gonna do that is prefer it to not be part of the Dangerfile, is not it's job. So either using octokit ourselves or find a way to reuse the danger cache after the danger step.

For now I think it's not worth the effort and could be just ruled as nice to have 😉

[AliSoftware]

Note that maybe there would be another way in case circle CI exposes the git target branch of that PR as part of env variables or something. In that case, no need for GH metadata, a git diff or git log with proper parameters could give us the list

[djbe]

https://circleci.com/docs/1.0/environment-variables/ it does (the commit branch and SHA)

[djbe]

git diff-tree --no-commit-id --name-only -r a19704f32e070f642e4f5c1fa6e19de4624e39f5 | grep "Tests/Expected"
works, but we need to be careful, there might be situations where we want to force compile all files.

[AliSoftware]

I'm not sure which env vars you're talking about, all the env vars I see contains the SHA1 and branch name… being tested, not the branch we try to merge into, does it?

So if the PR is trying to merge feature/foo into master we need both branch names (or SHA1s), and especially the name master, so we could run some git diff --name-only master when circle CI is running its tests on code pulled from the feature/foo branch, so that we can list what files would be different between the target branch (master) and the branch being tested (feature/foo) which wants to be merged into this target branch

And I guess those env vars contain the name of the branch being tested so feature/foo but not the name of the target branch master to compare against?

[djbe]

Ah what I meant was to only compile the files changed by a commit, but that would indeed not be enough now that I think about it:

• a previous commit broke the output and
• this commit doesn't touch the output files
• --> the broken output files won't be tested again.

The command you suggested works though, and we could add an extra check:

• on master, always test the full output files
• on a branch, only test the changed output files using git diff --name-only master

[AliSoftware]

Anyway I think this is probably an over optimisation at least for now, especially given that it feels too risky to me to miss some cases and not compile files that we should test anyway.

For example if the stub env for those files were to change we'd still need to compile everything to be sure. It would be easy to miss some cases and think it compiles (because of green status) while it doesn't

Better compile all tests and trust our CI even if it takes a little more time, rather than trying to avoid compiling some files and have false positive builds, which seem way more dangerous to me. I prefer a CI I can trust, and if we were gonna optimize something it would rather be compiling expected files in parallel rather than trying to compile only some of them.

[AliSoftware]

on a branch, only test the changed output files using git diff --name-only master

There's no guarantee all PRs are against master, so that assumption seems dangerous to me too (and that's why I was looking for an env var containing that info but doesn't seem there's one, and querying it via octokit doesn't seem worth it to me anyway as explained in my previous comment — too great a risk to miss an overlooked use case)

[djbe]

Yeah, let's just leave that be for what it is. I don't know if we can parallelize our builds though, not with the current plan we have.

[AliSoftware]

Same remarks for rakelib/lint.rake rakelib/utils.rake here than the ones in SwiftGen/SwiftGen#269 (review)

[AliSoftware]

Same comment as SwiftGen/StencilSwiftKit#28 (comment)

[AliSoftware]

Check for the podspec, not the Podfile