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

Test CI with OS X #136

Merged
merged 8 commits into from Sep 7, 2016

Conversation

Projects
None yet
2 participants
@karthikreddy09
Contributor

karthikreddy09 commented Sep 2, 2016

Add Mac OS X to travis CI builds

karthikreddy09 added some commits Sep 2, 2016

@saschazelzer

This comment has been minimized.

Show comment
Hide comment
@saschazelzer

saschazelzer Sep 3, 2016

Member

Awesome, having Mac OS supported by Travis CI will be great.

A few comments:

  • If you want to test CI config changes, just pushing the branch itself is enough and will trigger the CI hooks. You don't need a PR.
  • Some elements in the build matrix have addons: &valgrind and some don't. What is the purpose of the & ?
  • It is unfortunate that we need to expand the build matrix manually. When we re-enable the single-threaded build configuration, this will double the number of entries. But given my understanding of the Travis CI config file and the need for the MACOSX_DEPLOYMENT_TARGET environment variable, I couldn't think of a cleaner way :-/

Thanks for working on this!

Member

saschazelzer commented Sep 3, 2016

Awesome, having Mac OS supported by Travis CI will be great.

A few comments:

  • If you want to test CI config changes, just pushing the branch itself is enough and will trigger the CI hooks. You don't need a PR.
  • Some elements in the build matrix have addons: &valgrind and some don't. What is the purpose of the & ?
  • It is unfortunate that we need to expand the build matrix manually. When we re-enable the single-threaded build configuration, this will double the number of entries. But given my understanding of the Travis CI config file and the need for the MACOSX_DEPLOYMENT_TARGET environment variable, I couldn't think of a cleaner way :-/

Thanks for working on this!

@saschazelzer

This comment has been minimized.

Show comment
Hide comment
@saschazelzer

saschazelzer Sep 3, 2016

Member

Oh, and the Linux Clang configurations are not supported on Ubuntu Trusty. Clang just doesn't play well with libstdc++ from gcc with C++11 enabled, so I suggest to just remove these entries from the build matrix.

Member

saschazelzer commented Sep 3, 2016

Oh, and the Linux Clang configurations are not supported on Ubuntu Trusty. Clang just doesn't play well with libstdc++ from gcc with C++11 enabled, so I suggest to just remove these entries from the build matrix.

@karthikreddy09

This comment has been minimized.

Show comment
Hide comment
@karthikreddy09

karthikreddy09 Sep 3, 2016

Contributor

I noticed the push/branch CI nodes but never made the connection.

(&) is used to create anchor nodes in YAML. The same nodes can be later referenced using a (*). The anchor and reference nodes can be used to remove redundancy in the file. I haven't yet figured how to do it right in our yml file

Contributor

karthikreddy09 commented Sep 3, 2016

I noticed the push/branch CI nodes but never made the connection.

(&) is used to create anchor nodes in YAML. The same nodes can be later referenced using a (*). The anchor and reference nodes can be used to remove redundancy in the file. I haven't yet figured how to do it right in our yml file

karthikreddy09 added some commits Sep 3, 2016

Use node label and reference to condense the file
Signed-off-by: The Mathworks Inc <Roy.Lurie@mathworks.com>
@karthikreddy09

This comment has been minimized.

Show comment
Hide comment
@karthikreddy09

karthikreddy09 Sep 7, 2016

Contributor

@CppMicroServices/developers let me know if you are OK with merging this PR.

Contributor

karthikreddy09 commented Sep 7, 2016

@CppMicroServices/developers let me know if you are OK with merging this PR.

@saschazelzer

This comment has been minimized.

Show comment
Hide comment
@saschazelzer

saschazelzer Sep 7, 2016

Member

+1 for merging

This is really nice to have!

Member

saschazelzer commented Sep 7, 2016

+1 for merging

This is really nice to have!

@karthikreddy09 karthikreddy09 merged commit 710c4b9 into development Sep 7, 2016

4 checks passed

continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@karthikreddy09 karthikreddy09 deleted the ci-config-osx branch Sep 7, 2016

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