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

In Rails 5.2 passing default to #index_name_exists? was removed. #390

Merged
merged 7 commits into from
Jul 10, 2018

Conversation

biow0lf
Copy link
Contributor

@biow0lf biow0lf commented Nov 30, 2017

Changes:

  • Added rails 5.2 and rails master to travis build matrix
  • Wrap #index_name_exists? with rails version check

Rails 5.2 CLIENT=redis test suit fails. Need some more love. ❤️ I will add more commits for this later in this PR, or open another PR.

@aried3r
Copy link
Member

aried3r commented Jan 18, 2018

Thanks @biow0lf! Looks good so far. Do you think you can get 5.2 to work with redis?

/cc @Tonkpils

@aried3r
Copy link
Member

aried3r commented Feb 23, 2018

Hi, could you rebase this branch? There's a few fixes in master that should help with some of the CI failures. Then we can have another look :)

@biow0lf
Copy link
Contributor Author

biow0lf commented Feb 23, 2018

Sure, I will rebase it. Also, I want update update modis gem to work with rails 5.2. But, I need some time for this.

@Tonkpils
Copy link

Let me know if there's anything I can do to move this effort forward. I'd be happy to 👀 or pair on this work 😄

@aried3r
Copy link
Member

aried3r commented Apr 11, 2018

Hi, I rebased this branch and now only the tests with Rails 5.2 with fail. Not sure yet what's causing them.

@aried3r
Copy link
Member

aried3r commented Apr 19, 2018

I think this PR is fine for AR as a backend, since the tests work. But it seems the redis errors are unrelated to that, since they don't use the migrations.

Might be worth merging this and working on the redis issue separately. What do you think?

@biow0lf
Copy link
Contributor Author

biow0lf commented Apr 19, 2018

Might be worth merging this and working on the redis issue separately. What do you think?

redis backend broken due broken support of rails 5.2 in gem modis.

@soulfly
Copy link

soulfly commented May 3, 2018

We face the same issue with modis support in rails 5.2
rpush/modis#13

Does anybody have a clue when this can be done?

We can't update our rails app from 4.2 to 5.2 because of this issue with modis in rpush

@aried3r
Copy link
Member

aried3r commented May 23, 2018

@soulfly, no idea, sadly. I don't use modis at all. Are there actively maintained alternatives out there we could switch there in case modis does not receive an update?

I'm thinking about accepting this PR since it seems to work with ActiveRecord and writing a notice in the README that for now, it doesn't work with ~~~MongoDB~~~ Redis.

@soulfly
Copy link

soulfly commented May 23, 2018

@aried3r

it doesn't work with MongoDB.

You mean Redis

not sure about alternatives, Modis is something that integrated with ActiveRecord API so you do not need to learn any new API with it

@OL-Mark do you have any thoughts?

@aried3r
Copy link
Member

aried3r commented May 23, 2018

You mean Redis

Yes, you're right, I've corrected it in my comment, thanks!

@aried3r
Copy link
Member

aried3r commented Jul 10, 2018

Sorry for taking so long. I'll have another look and add a notice to the README that Redis support currently doesn't exist for Rails 5.2 with Modis.

Hope to get it out soon, thanks for the PR @biow0lf and everyone else for their help!

Adrian1707 pushed a commit to Adrian1707/rpush that referenced this pull request Apr 24, 2024
In Rails 5.2 passing default to #index_name_exists? was removed.
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.

None yet

4 participants