Skip to content

Start CI, fixing an error on Ruby 2.5. #68

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

Conversation

junaruga
Copy link
Contributor

@junaruga junaruga commented Feb 8, 2018

No change for Gemfile.lock.

  • Update a template file for Ruby 2.5.0 ERB syntax.
  • Add Travis CI.

According to latest revert [1] by an error on openshift/origin [2], the reason looks related to new version of mysql2 and activerecord.

So, let's do a baby step.

Ideally one day, it is good to add unit test cases for run.sh and to be run by openshift/origin, to this repository's CI.
What I want is to run run.sh on Ruby 2.5.

Running run.sh on Ruby 2.5, that shows warnings like this.

/opt/app-root/src/bundle/ruby/2.5.0/gems/sinatra-1.4.5/lib/sinatra/base.rb:1217: warning: constant ::Fixnum is deprecated

But I think that this PR is still okay for the first step.

[1] #67
[2] openshift/origin#18522 (comment)

* Update a template file for Ruby 2.5.0 ERB syntax.
* Add Travis CI.
@0xmichalis
Copy link

@bparees we need to start gating PRs in this repo with the Origin e2e job

@0xmichalis
Copy link

cc @stevekuznetsov @mfojtik

@bparees
Copy link
Contributor

bparees commented Feb 8, 2018

@Kargakis that's basically impossible. the origin CI tests directly clone this repo from master to run their tests. There is no way to run those tests against the proposed changes, without significant refactoring of those tests.

@0xmichalis
Copy link

The test needs to be refactored to not make assumptions about cloning anyway. I think this will be enabled by @stevekuznetsov's work on the new job api.

@bparees
Copy link
Contributor

bparees commented Feb 8, 2018

@Kargakis cloning is part of what we are testing. we have to test our build functionality and part of that is "clone from a remote repo and build it"

@0xmichalis
Copy link

I see. Then, I guess this repo needs its own e2e test.

@bparees
Copy link
Contributor

bparees commented Feb 8, 2018

locally tested this, confirmed the app deploys and the DB is accessible. merging.

@bparees bparees merged commit faccd39 into openshift:master Feb 8, 2018
@junaruga
Copy link
Contributor Author

junaruga commented Feb 8, 2018

@bparees thanks for your quick testing. That helps me.

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

Successfully merging this pull request may close these issues.

3 participants