Skip to content

Support for Rails 5 #41

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

Closed
wants to merge 8 commits into from
Closed

Support for Rails 5 #41

wants to merge 8 commits into from

Conversation

elektronaut
Copy link

Hi! I'd really love to have this gem updated for Rails 5.

I've taken the work done by @schuylr in #32, rebased onto master and fixed the tests so that they're running on the final 5.0 release without deprecations.

This bumps the version to 2.0.0 and drops support for Rails 4.

@cguyer
Copy link

cguyer commented Oct 3, 2016

Can we get this merged in. Also @elektronaut happen to have a fork of this public till merge?

@elektronaut
Copy link
Author

@cguyer sure, I've been using this:

gem "actionpack-page_caching",
    git: "https://github.com/kord-as/actionpack-page_caching",
    branch: "rails5"

@RogerE
Copy link

RogerE commented Oct 11, 2016

There does not seem to be much focus on this gem from Rails core. Understandable of course, they probably don't use it and have lots if other things to deal with. Maybe it could be spun out from the Rails umbrella like acts_as_list and will_paginate?

@elektronaut
Copy link
Author

The only real change here is renaming an after_filter callback to after_action, the rest is just test instrumentation and version housekeeping. Please let me know if there's anything else I can do to facilitate a release.

FWIW, I've still got several projects depending on this, and I'm willing to throw my hat in the ring and maintain it as-is if the Rails core team wants to abandon it.

@@ -2,5 +2,5 @@ source 'https://rubygems.org'

gemspec

gem 'rails'
gem 'rails', '>= 5.0.0beta1.0'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need of this.

- 2.3.1
gemfile:
- Gemfile
- gemfiles/Gemfile-4-0-stable
- gemfiles/Gemfile-4-1-stable
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should keep this if we are not dropping support to it

@@ -2,19 +2,13 @@ language: ruby
before_install:
- gem install bundler
rvm:
- 1.9.3
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why those ruby versions are being removed?

@schuylr
Copy link

schuylr commented Oct 11, 2016

@rafaelfranca I noticed your diff comments which are mostly on my legwork - unfortunately we can't support both Rails 4.x and Rails 5 in the same codebase. Subsequently, Rails 5 doesn't support the older Ruby versions that you're inquiring as to why they're being removed.

See these comments:

#31 (comment)
#32 (comment)

Lastly, the >= 5.0.0beta1.0 was when Rails was an official beta and not installable without this line. I agree that it is now unnecessary with the release of 5.0.

I would follow @eileencodes's suggested strategy, and derive two supported branches with a bump to v2 for this gem.

@rafaelfranca
Copy link
Member

I think we can support Rails 4 and older versions of ruby. There is no changes in the code base only in the test suite. In that case is possible to only run Rails 5 with the supported versions and it is possible to conditionally define code depending in the Rails version to make the test suite pass with 4.x.

@elektronaut
Copy link
Author

I agree, Rails 4.x support is totally possible.

I've got 4.0, 4.1, 4.2 and 5.0 working locally, let's see what the CI says.

@elektronaut
Copy link
Author

That should do the trick. I've changed the version number to a point release, since that would be more appropriate.

@romenigld
Copy link

Thank's elektronaut!! works fine for me:

gem "actionpack-page_caching",
    git: "https://github.com/kord-as/actionpack-page_caching",
    branch: "rails5"

@pixeltrix
Copy link
Contributor

@elektronaut sorry, I couldn't use this PR but hopefully you'll be happy now that Rails 5.0 support has been officially added - thanks for your contribution.

@pixeltrix pixeltrix closed this Nov 7, 2016
@RogerE
Copy link

RogerE commented Nov 11, 2016

Will there be a new gem version released for this?

@pixeltrix
Copy link
Contributor

@RogerE Yes - I'm also tidying up the action caching gem and will be releasing a new version of that too.

@RogerE
Copy link

RogerE commented Nov 11, 2016

@pixeltrix excellent, thanks!

@tomhughes
Copy link

Is there any news on getting the new version out?

@sighmon
Copy link

sighmon commented Dec 13, 2016

@tomhughes I've pointed my Gemfile directly at this git repo to test it out with Rails 5.

gem 'actionpack-page_caching', git: 'https://github.com/rails/actionpack-page_caching.git'

@pixeltrix
Copy link
Contributor

@tomhughes I'll be getting releases out for this and action gems out over Christmas

@enrico
Copy link

enrico commented Dec 18, 2016

@pixeltrix Any chance we will also get a new gem version for rails/rails-observers ?
actionpack-page_caching and rails-observers are the only 2 entries in my Gemfile not requiring a specific version and instead pointing to git directly...

@pixeltrix
Copy link
Contributor

@enrico not sure about rails-observers - will have to check what's involved in that

@jaredmdobson
Copy link

Any help needed with this?

@pixeltrix
Copy link
Contributor

@onesupercoder actionpack-page_caching or rails-observers? No for the former and yes for the latter 😄

I'm just writing a blog post for the release of this and the action caching gem - out at the weekend.

@jaredmdobson
Copy link

jaredmdobson commented Jan 19, 2017 via email

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

Successfully merging this pull request may close these issues.