Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix failing build. #98

Merged
merged 2 commits into from
Feb 15, 2013
Merged

Fix failing build. #98

merged 2 commits into from
Feb 15, 2013

Conversation

cmeiklejohn
Copy link
Contributor

Since compile relies on deps being present, ensure they are.

@eproxus
Copy link
Owner

eproxus commented Feb 15, 2013

Thanks! I will have look closer at this, because right now builds seems to be failing because of some issue with Travis and node cookies. Will talk to the Travis guys about this first...

@horkhe Could remember that the dependencies are needed only for testing, any input on this?

@cmeiklejohn
Copy link
Contributor Author

One thing I noticed, is that the all task calls compile, which needs the deps, according to the Makefile. I see it didn't fix the build, but it definitely solves the problem of running make locally and getting a failure.

@eproxus
Copy link
Owner

eproxus commented Feb 15, 2013

It's test that has dependencies, not build. But yes, make all runs test so it should include get-deps as well.

Reported the build issues to Travis CI:

Will merge this now since the builds work locally.

eproxus added a commit that referenced this pull request Feb 15, 2013
@eproxus eproxus merged commit 27ed56a into eproxus:develop Feb 15, 2013
@cmeiklejohn cmeiklejohn deleted the csm-fix-build branch February 15, 2013 17:48
@cmeiklejohn
Copy link
Contributor Author

Thank you!

@eproxus
Copy link
Owner

eproxus commented Feb 15, 2013

Build is now successful: https://travis-ci.org/eproxus/meck/builds/4821395 (including the fixes 0b17a07 - 7cbb389)

@horkhe
Copy link
Contributor

horkhe commented Feb 18, 2013

Sorry could not reply immediately, and yes, the dependencies are needed for testing only. But you have already figured that out on your own :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants