terhechte / SourceKittenDaemon

Swift Auto Completions for any Text Editor
MIT License
529 stars 37 forks source link

Check whether the commit can be built on Travis CI. #36

Closed mitsuse closed 8 years ago

mitsuse commented 8 years ago

This is replacement of #34 for git rebase.

I tried building SourceKittenDaemon on Travis CI. It may be useful to guarantee build passes with clean environment and to automate deployment in future.

Please check the build status (This is a temporary link):

nathankot commented 8 years ago

LGTM ;) @terhechte will need to setup the Travis side of things first

nathankot commented 8 years ago

@mitsuse please see #37, I've added make test which builds and runs the tests which will be a better command for Travis :)

Also see a489566, I've removed the --recursive flag on submodule update --init because I don't think it's necessary - this might resolve the issue you were having with Nimble?

nathankot commented 8 years ago

BTW, there is currently 1 failing test but I haven't had time to look into it

mitsuse commented 8 years ago

@nathankot Thanks! I added make test to the build script for Travis CI after merging master.

Also see a489566, I've removed the --recursive flag on submodule update --init because I don't think it's necessary - this might resolve the issue you were having with Nimble?

I have not read source codes of Carthage yet, The option --recurvie seems to be required because build failed without it on Travis CI:

Please try to execute make dist in a local repository just after git clone. It might reproduce an error.

nathankot commented 8 years ago

Hmmm the error doesn't look submodule-related:

/Users/travis/Library/Developer/Xcode/DerivedData/Commandant-ajotfymovaoyancuxpxppynicbtt/Build/Intermediates/Commandant.build/Release/Commandant.build/unextended-module.modulemap:2:19: error: umbrella header 'Commandant.h' not found
<unknown>:0: error: could not build Objective-C module 'Commandant'

I just pushed 54efd9f, which make dist's correctly on a fresh clone, can you see if it succeeds on Travis?

terhechte commented 8 years ago

@nathankot I've actually never done that before 😳 What specifically do I need to do? Just sign up on travis-ci.org and add the project? Or are there specific configurations I need to do?

nathankot commented 8 years ago

@terhechte yup, pretty much just sign up to TravisCI, add the project and setup the github hook

mitsuse commented 8 years ago

@nathankot

Hmmm the error doesn't look submodule-related:

/Users/travis/Library/Developer/Xcode/DerivedData/Commandant-ajotfymovaoyancuxpxppynicbtt/Build/Intermediates/Commandant.build/Release/Commandant.build/unextended-module.modulemap:2:19: error: umbrella header 'Commandant.h' not found
<unknown>:0: error: could not build Objective-C module 'Commandant'

I agree and 543fd9f was passed but a test failed as you said before.

This works successfully to build, but I can't judge whether it is correct or not to use carthage bootstrap in this case... Although git submodule update --init --recursive and carthage build also work, what is the difference between them and carthage bootstrap except that carthage bootstrap uses Carthfile.resolved?

nathankot commented 8 years ago

@mitsuse

Hey so the build is working now? =D

From what I know, the differences would be:

terhechte commented 8 years ago

Ok, I've set up travis, so (if I understand things correctly) once we merge this PR, and the travis.yml is added to the project, travis should automatically pick up the new travis.yml file and run a build?

mitsuse commented 8 years ago

@nathankot @terhechte Thanks!

I think this pull-request is ready to be merged. A test case has failed before this, so it should be fixed with another pull-request.

The build also works fine except for the formula. Homebrew executes git submodule update --init --recursive after checking-out by default. For a discussion about build, maybe I will create a new issue,

nathankot commented 8 years ago

Yup!

nathankot commented 8 years ago

Just realised I made a pretty vague comment (too many drinks last night lol.) I meant yup its ready to merge, and we can continue the homebrew discussion on #28

nathankot commented 8 years ago

I've fixed the regression on 4aab053 so I will merge this now!