-
Notifications
You must be signed in to change notification settings - Fork 39
Update .travis.yml to solve PostgreSQL issues with new base images #32
base: master
Are you sure you want to change the base?
Conversation
Travis CI updated the base images so the hacky PG 10 install is no longer needed and on top of that is failing the builds.
6181c31
to
7fd965f
Compare
Looks like I need to lint the code and fix some violations in style. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good to me!
@tdmalone This looks good to me, however there's something wrong with the CI checks (how do we have two separate Travis CI instances?) so I can't merge it. I'll leave it to you to sort this out. |
The style issues aren't because of your changes, so no need to fix them. (in this PR) |
There are style issues that were not in my changes but not the "todo" warnings. The build was failing the style check, I suspect a newer version of eslint is used with more checkers? Regardless, without the style fixed this build will not pass Travis CI check and will not be mergable. I can try to find out where the second CI build is triggered from, not really sure where that would be to be honest. |
The build isn't failing, I don't believe linter warnings actually fail it.
That's a job for Tim when he gets back. I'd just merge this manually, but I'm only 99% certain that this is correct and I'd like another set of eyes on it. |
The style check was failing on my other PR due to my changes. This comment was out of place for this PR. Sorry for the confusion. For what it is worth, I made the changes based on the Travis CI documentation. In fact I cut/pasted the recipe from the docs verbatim. |
This is the only project I am involved with that uses Travis CI, so I lack the confidence to click the big "Merge" button, simple as that =) |
Not a problem, I am in no rush to get this merged. I use it in my instance from my github anyway. It would help getting this merged to decouple the other two PR that depend on it and include this commit to get them building. I will let @tdmalone decide how to handle the 3 PR, I can decouple them into two separate changes plus this one if this one is merged first to fix the cause of the build failures due to the platform change. |
I believe this explains the two builds we see. As the master branch is not buildable I think we will get this cleared after this PR is merged. This would be the behavior from Travis testing both the target merge and the PR branch in isolation. I was wrong, this is the real cause and it is something @tdmalone has to change in the Github branches configuration. The link has the full details. |
I can't, it won't let me due to the CI failure. @tdmalone? |
This might require merge from git instead of the Github GUI. If this gets merged I believe the CI builds will start working and enable the rest of the PR to be ready for merge. |
Travis CI updated the base images so the hacky PG 10 install is no
longer needed and on top of that is failing the builds.