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

Enable continuous integration #16

Conversation

carlschroedl
Copy link
Contributor

@carlschroedl carlschroedl commented Nov 5, 2017

Addresses #15.

Follows Travis CI's recommendations for building C++ 11 and beyond.

Although the basic build steps succeed, The Travis CI job currently fails with an import error while executing the tests. This is probably because I don't know how to get the tests to execute successfully. It is also possible that the tests are broken.

We could try some combination of:

  • update the script section of the .travis.yml so that it correctly execute the tests
  • fix the tests to resolve errors

Alternatively, in the short term we could change the script section to true so that Travis CI verifies that the basic build steps succeed without verifying that the tests run correctly.

Do y'all have any ideas on how to proceed?

@shaldengeki
Copy link

shaldengeki commented Nov 19, 2017

I believe the Travis job fails because the tests are broken -- they are trying to refer to modules which were removed in this commit: a85574c#diff-f09c9d1e1545e0e7689558c53ec18520

It appears that compactnesslib has superceded the functionality originally provided by the removed modules. In that case, the tests probably need updating.

@carlschroedl
Copy link
Contributor Author

Aha! Makes sense. Thank you for tracking down that commit.

@r-barnes
Copy link
Collaborator

GCC 4.9 seems pretty old at this point. I've developed compactnesslib in 6 and 7; I'm not sure how backwards compatible that will be. Any chance you can bump the Travis version?

@r-barnes r-barnes self-assigned this Nov 20, 2017
@carlschroedl
Copy link
Contributor Author

Sure! Will do!

@carlschroedl
Copy link
Contributor Author

@r-barnes I've increased the gcc and g++ versions. As a temporary fix I have also stopped the Travis build from failing by telling it to skip the tests. I'm hoping we can tackle that in #17 .

@carlschroedl
Copy link
Contributor Author

Here's a peek at the passing build:
https://travis-ci.org/carlschroedl/python-mander/builds/309249253

Would you say this PR is getting to mergealicious territory, or does it need a few more minutes in the metaphorical code oven? :)

@msarahan
Copy link
Member

msarahan commented Mar 2, 2018

Thanks for working on this @carlschroedl. I've found CircleCI to be quite a bit faster than Travis, and I'm working on establishing that for all of the projects before or as part of the coming hackathon in March. I'm inclined to close this PR because the work here won't transfer, but if you'd prefer that I merge it to give you credit for it, then make my changes on top of yours, I'm cool with that too. What would you prefer?

@msarahan
Copy link
Member

msarahan commented Mar 2, 2018

#19 implements circleci integration. I'll wait a few days to hear from @carlschroedl before merging that.

@carlschroedl
Copy link
Contributor Author

@msarahan I believe it was Scooby-Doo who once said "It is amazing what you can accomplish if you do not care who gets the credit." ;)

The important thing is that this project has some form of CI. If you have a better solution -- particularly if it is an approach shared across many gerrymandr repos --then let's go for it.

@ghost ghost removed the in progress label Mar 5, 2018
@msarahan
Copy link
Member

msarahan commented Mar 5, 2018

I wasn't aware of that quote. It's very true, but it's always better to figure out if someone has that attitude before you toss their work, rather than after.

Thanks again for pointing the organization in the right direction (using CI), and for your other work on this effort.

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.

4 participants