Rails 4.2 compatibility #3153

Merged
merged 21 commits into from Oct 3, 2014

Conversation

Projects
None yet
@lucasmazza
Contributor

lucasmazza commented Aug 17, 2014

This Pull Request aims to bring back compatibility with Rails 4.2/master since rails/rails@dc3f25c has changed the @scope data structure from the router, without losing compatibility with existing versions of Rails where this object is a Hash.

In the matter of Rails 4.2, we also have this deprecation warning:

DEPRECATION WARNING: defining a route where `to` is a controller without an action is deprecated.  Please change "to: :users/omniauth_callbacks" to "controller: :users/omniauth_callbacks". (called from devise_omniauth_callback at /Users/lucas/code/ptec/devise/lib/devise/rails/routes.rb:436)

Changing the router code to use controller: controllers[:omniauth_callbacks] leads to the following error:

/opt/rubies/2.1.2/lib/ruby/gems/2.1.0/bundler/gems/rails-d5be08347fb7/actionpack/lib/action_dispatch/routing/mapper.rb:263:in `block (2 levels) in check_controller_and_action': 'users/auth/:action' is not a supported controller name. This can lead to potential routing problems. See http://guides.rubyonrails.org/routing.html#specifying-a-controller-to-use (ArgumentError)
  • Fix router monkeypatch.
  • Remove OmniAuth route warning.
  • Deal with respond_with removal.
  • Reorganize the project Gemfiles.
  • :shipit:
lib/devise/rails/routes.rb
@@ -445,16 +445,15 @@ def devise_omniauth_callback(mapping, controllers) #:nodoc:
DEVISE_SCOPE_KEYS = [:as, :path, :module, :constraints, :defaults, :options]
def with_devise_exclusive_scope(new_path, new_as, options) #:nodoc:
- old = {}
- DEVISE_SCOPE_KEYS.each { |k| old[k] = @scope[k] }

This comment has been minimized.

@carlosantoniodasilva

carlosantoniodasilva Aug 18, 2014

Collaborator

Will you still need this constant, then?

@carlosantoniodasilva

carlosantoniodasilva Aug 18, 2014

Collaborator

Will you still need this constant, then?

This comment has been minimized.

@lucasmazza

lucasmazza Aug 18, 2014

Contributor

🏃

@lucasmazza

lucasmazza Aug 18, 2014

Contributor

🏃

@carlosantoniodasilva

This comment has been minimized.

Show comment
Hide comment
@carlosantoniodasilva

carlosantoniodasilva Aug 18, 2014

Collaborator

Seems good to me 👍

Collaborator

carlosantoniodasilva commented Aug 18, 2014

Seems good to me 👍

@Fudoshiki

This comment has been minimized.

Show comment
Hide comment
@Fudoshiki

Fudoshiki Aug 19, 2014

Contributor

Merge this with master branch

Contributor

Fudoshiki commented Aug 19, 2014

Merge this with master branch

@lucasmazza

This comment has been minimized.

Show comment
Hide comment
@lucasmazza

lucasmazza Aug 20, 2014

Contributor

I would really like to remove the warning before merging this (@josevalim, any ideas?), and probably deal with #3157 before releasing a 4.2.0 compatible version.

Once we are good to go this will be merged, don't worry :).

Contributor

lucasmazza commented Aug 20, 2014

I would really like to remove the warning before merging this (@josevalim, any ideas?), and probably deal with #3157 before releasing a 4.2.0 compatible version.

Once we are good to go this will be merged, don't worry :).

@lucasmazza lucasmazza changed the title from Bring back rails master to Rails 4.2 compatibility Aug 20, 2014

@lucasmazza

This comment has been minimized.

Show comment
Hide comment
@lucasmazza

lucasmazza Aug 20, 2014

Contributor

@carlosantoniodasilva @josevalim another detail that we need to address: Devise does a hard use of respond_with, so this means that we now depend on the responders. Should we have a more specialized error message or let rails raise the "The controller-level 'respond_to' feature has been extracted..." one?

Contributor

lucasmazza commented Aug 20, 2014

@carlosantoniodasilva @josevalim another detail that we need to address: Devise does a hard use of respond_with, so this means that we now depend on the responders. Should we have a more specialized error message or let rails raise the "The controller-level 'respond_to' feature has been extracted..." one?

lucasmazza added some commits Aug 17, 2014

Use `[]=` instead of `merge!` to mutate the current Router scope.
This was broken in Rails 4.2.0+ because the `@scope` object is no longer a Hash
but an internal structure that supports a better override/rollback flow for cases
like this. If we would only support Rails 4.2, this method could be something
like this:

```ruby
def with_devise_exclusive_scope(new_path, new_as, options)
  overrides = { as: new_as, path: new_path, module: nil }
  overrides.merge!(options.slice(:constraints, :defaults, :options))

    @scope = @scope.new(overrides)
  yield
ensure
  @scope = @scope.parent
end
```
@lucasmazza

This comment has been minimized.

Show comment
Hide comment
@lucasmazza

lucasmazza Aug 20, 2014

Contributor

Updated the whole thing, testing against the released 4.2.0.beta1 gem. I've noticed several intermittent errors when running the test suite locally multiple times and we need to take a look on that.

Contributor

lucasmazza commented Aug 20, 2014

Updated the whole thing, testing against the released 4.2.0.beta1 gem. I've noticed several intermittent errors when running the test suite locally multiple times and we need to take a look on that.

@josevalim

This comment has been minimized.

Show comment
Hide comment
@josevalim

josevalim Aug 20, 2014

Member

Those changes look great. Will Devise depend just on Rails 4.2 then? It kinda sucks that Rails is basically forcing us to do backwards incompatible changes.

I am also fine with depending on responders, at least for now, because most Rails devs will still expect this kind of stuff to just work.

Member

josevalim commented Aug 20, 2014

Those changes look great. Will Devise depend just on Rails 4.2 then? It kinda sucks that Rails is basically forcing us to do backwards incompatible changes.

I am also fine with depending on responders, at least for now, because most Rails devs will still expect this kind of stuff to just work.

@carlosantoniodasilva

This comment has been minimized.

Show comment
Hide comment
@carlosantoniodasilva

carlosantoniodasilva Aug 20, 2014

Collaborator

We could try to move on to respond_to block rather than respond_with in most cases, just not sure it's totally worth it anyway. The other changes doesn't seem to be totally back imcompatible, they should work the same way on Rails 4.x apparently, but I might be wrong. Looking good 👍

Collaborator

carlosantoniodasilva commented Aug 20, 2014

We could try to move on to respond_to block rather than respond_with in most cases, just not sure it's totally worth it anyway. The other changes doesn't seem to be totally back imcompatible, they should work the same way on Rails 4.x apparently, but I might be wrong. Looking good 👍

@@ -435,26 +435,23 @@ def devise_omniauth_callback(mapping, controllers) #:nodoc:
match "#{path_prefix}/:action/callback",
constraints: { action: providers },
- to: controllers[:omniauth_callbacks],
+ to: "#{controllers[:omniauth_callbacks]}#:action",

This comment has been minimized.

@josevalim

josevalim Aug 20, 2014

Member

Isn't it better to just replace to: by controller:? Would that get rid of the warning?

@josevalim

josevalim Aug 20, 2014

Member

Isn't it better to just replace to: by controller:? Would that get rid of the warning?

This comment has been minimized.

@lucasmazza

lucasmazza Aug 20, 2014

Contributor

Nope, it crashes the app with the second message I've posted on this pull request description.

@lucasmazza

lucasmazza Aug 20, 2014

Contributor

Nope, it crashes the app with the second message I've posted on this pull request description.

@lucasmazza

This comment has been minimized.

Show comment
Hide comment
@lucasmazza

lucasmazza Aug 20, 2014

Contributor

I don' think that rewriting the respond_with is a good path, and having a fixed dependency on responders will force us to drop support for Rails 4.0/4.1.

I'm thinking of going with an exception for now (something like 'Devise depends on the respond_to/respond_with API that was extracted to the responders gem. Please add gem "responders", "~> 2.0"' to your Gemfile') so we can support multiple versions with Devise 3.x and then look for a major release that drops this backwards support - something like Devise 4 is only for Rails 4.2+.

Does it sounds too complicated?

Contributor

lucasmazza commented Aug 20, 2014

I don' think that rewriting the respond_with is a good path, and having a fixed dependency on responders will force us to drop support for Rails 4.0/4.1.

I'm thinking of going with an exception for now (something like 'Devise depends on the respond_to/respond_with API that was extracted to the responders gem. Please add gem "responders", "~> 2.0"' to your Gemfile') so we can support multiple versions with Devise 3.x and then look for a major release that drops this backwards support - something like Devise 4 is only for Rails 4.2+.

Does it sounds too complicated?

@trinode

This comment has been minimized.

Show comment
Hide comment
@trinode

trinode Aug 20, 2014

@lucasmazza this sounds good to me, I've been limited before by gem dependencies of libraries, when a perfectly good API compatible alternative exists (eg cancan dependency and I wanted to use cancancan), this approach will allow us to use super_awesome_mega_reponders.gem (which I made up) if it provided all the necessary functions I presume?

trinode commented Aug 20, 2014

@lucasmazza this sounds good to me, I've been limited before by gem dependencies of libraries, when a perfectly good API compatible alternative exists (eg cancan dependency and I wanted to use cancancan), this approach will allow us to use super_awesome_mega_reponders.gem (which I made up) if it provided all the necessary functions I presume?

@lucasmazza

This comment has been minimized.

Show comment
Hide comment
@lucasmazza

lucasmazza Aug 20, 2014

Contributor

@trinode sort of. Once we add responders as a fixed dependency on Devise this won't be so easy, but I don't know if there any responders alternative out there. The main goal is to keep compatibility between Devise and multiple Rails versions.

Contributor

lucasmazza commented Aug 20, 2014

@trinode sort of. Once we add responders as a fixed dependency on Devise this won't be so easy, but I don't know if there any responders alternative out there. The main goal is to keep compatibility between Devise and multiple Rails versions.

@trinode

This comment has been minimized.

Show comment
Hide comment
@trinode

trinode Aug 20, 2014

@lucasmazza If it achieves the goal of older version compatibility it can only be a good thing, and like mentioned before, dropping 4.1 and below (or perhaps anything below 5 if there are features for Devise to take advantage of in Rails 5) from the next major version. Perhaps update the docs to mention to include it since it doesn't appear to be a default include in a new app, so long as it's clear in the exception output I don't think it'd cause too much friction.

trinode commented Aug 20, 2014

@lucasmazza If it achieves the goal of older version compatibility it can only be a good thing, and like mentioned before, dropping 4.1 and below (or perhaps anything below 5 if there are features for Devise to take advantage of in Rails 5) from the next major version. Perhaps update the docs to mention to include it since it doesn't appear to be a default include in a new app, so long as it's clear in the exception output I don't think it'd cause too much friction.

@danielmorrison danielmorrison referenced this pull request in LocalOrbit/localorbit Aug 20, 2014

Closed

Rails 4.2 #1200

1 of 3 tasks complete
@rafaelfranca

This comment has been minimized.

Show comment
Hide comment
@rafaelfranca

rafaelfranca Aug 20, 2014

Collaborator

I'd go with the path of avoiding using responders, unless we have behaviour coupled in the gem.

Collaborator

rafaelfranca commented Aug 20, 2014

I'd go with the path of avoiding using responders, unless we have behaviour coupled in the gem.

@edpaget edpaget referenced this pull request in zooniverse/Panoptes Aug 20, 2014

Closed

Update to Rails 4.2 #140

@lucasmazza

This comment has been minimized.

Show comment
Hide comment
@lucasmazza

lucasmazza Aug 20, 2014

Contributor

@rafaelfranca I think we are too coupled to drop it right now.

Contributor

lucasmazza commented Aug 20, 2014

@rafaelfranca I think we are too coupled to drop it right now.

@rafaelfranca

This comment has been minimized.

Show comment
Hide comment
@rafaelfranca

rafaelfranca Aug 20, 2014

Collaborator

I see. Can't we just say we depends on responders without specifying any version and let bundler resolve it correctly to different Rails versions? responders 2.0 requires 4.2 so bundler will never let 1.x be installed. Same when you are using 4.1.

Collaborator

rafaelfranca commented Aug 20, 2014

I see. Can't we just say we depends on responders without specifying any version and let bundler resolve it correctly to different Rails versions? responders 2.0 requires 4.2 so bundler will never let 1.x be installed. Same when you are using 4.1.

@lucasmazza

This comment has been minimized.

Show comment
Hide comment
@lucasmazza

lucasmazza Aug 20, 2014

Contributor

👍 for that If we are ok with shipping responders for free for everyone 😄

Contributor

lucasmazza commented Aug 20, 2014

👍 for that If we are ok with shipping responders for free for everyone 😄

@rafaelfranca

This comment has been minimized.

Show comment
Hide comment
@rafaelfranca

rafaelfranca Aug 20, 2014

Collaborator

Yes, there is no problem on shipping responders for free. It doesn't inject code automatically anyway.

Collaborator

rafaelfranca commented Aug 20, 2014

Yes, there is no problem on shipping responders for free. It doesn't inject code automatically anyway.

@lucasmazza

This comment has been minimized.

Show comment
Hide comment
@lucasmazza

lucasmazza Aug 20, 2014

Contributor

So I'll cut a 3.3.1 release for #3157 and 3.4.0.beta for Rails 4.2, and look through the test failures later.

Contributor

lucasmazza commented Aug 20, 2014

So I'll cut a 3.3.1 release for #3157 and 3.4.0.beta for Rails 4.2, and look through the test failures later.

@trinode

This comment has been minimized.

Show comment
Hide comment
@trinode

trinode Aug 22, 2014

When you release 3.4.0beta I can happily test it out on a fairly complex app (migrated to 4.2 beta from Rails 4.1), do you know what errors you were seeing intermittently so I know what to check for?

trinode commented Aug 22, 2014

When you release 3.4.0beta I can happily test it out on a fairly complex app (migrated to 4.2 beta from Rails 4.1), do you know what errors you were seeing intermittently so I know what to check for?

@lucasmazza

This comment has been minimized.

Show comment
Hide comment
@lucasmazza

lucasmazza Aug 22, 2014

Contributor

@trinode if you point your app to this branch I believe it will work as expected but I currently don't have any real world app to test Devise with. I know there are some inconsistencies with our test suite but if you manage to run your app against the branch and report how it goes that would be awesome ❤️ 💚 💙 💛 💜

Contributor

lucasmazza commented Aug 22, 2014

@trinode if you point your app to this branch I believe it will work as expected but I currently don't have any real world app to test Devise with. I know there are some inconsistencies with our test suite but if you manage to run your app against the branch and report how it goes that would be awesome ❤️ 💚 💙 💛 💜

@trinode

This comment has been minimized.

Show comment
Hide comment
@trinode

trinode Aug 23, 2014

@lucasmazza Totally happy to give it a go, I assume I use these lines for the Gemfile?:-

gem 'devise', :git => 'https://github.com/plataformatec/devise.git', :branch => 'lm-rails-4-2'
gem 'responders'

And then that'll be ready to go?

trinode commented Aug 23, 2014

@lucasmazza Totally happy to give it a go, I assume I use these lines for the Gemfile?:-

gem 'devise', :git => 'https://github.com/plataformatec/devise.git', :branch => 'lm-rails-4-2'
gem 'responders'

And then that'll be ready to go?

@trinode

This comment has been minimized.

Show comment
Hide comment
@trinode

trinode Aug 23, 2014

@lucasmazza It seems to be running fine, I don't use many advanced features of device though, so I can't say its 100% I spent most of my time working out what the heck happened to my CSS (sass 3.4 update is not happy with Foundation 5!)

trinode commented Aug 23, 2014

@lucasmazza It seems to be running fine, I don't use many advanced features of device though, so I can't say its 100% I spent most of my time working out what the heck happened to my CSS (sass 3.4 update is not happy with Foundation 5!)

@john

This comment has been minimized.

Show comment
Hide comment
@john

john Aug 23, 2014

👍 , switched to the 4.2 branch and can auth in to Twitter and Dropbox. fwiw I didn't need the responders gem--the relevant bits from my Gemfile:

gem 'rails', '4.2.0.beta1'
gem 'devise', git: 'https://github.com/plataformatec/devise.git', :branch => 'lm-rails-4-2'

Psyched to see this get merged, thank you!

john commented Aug 23, 2014

👍 , switched to the 4.2 branch and can auth in to Twitter and Dropbox. fwiw I didn't need the responders gem--the relevant bits from my Gemfile:

gem 'rails', '4.2.0.beta1'
gem 'devise', git: 'https://github.com/plataformatec/devise.git', :branch => 'lm-rails-4-2'

Psyched to see this get merged, thank you!

@gdiggs

This comment has been minimized.

Show comment
Hide comment
@gdiggs

gdiggs Aug 24, 2014

👍 seems to be working correctly for me too with the 4.2.0.beta1

gdiggs commented Aug 24, 2014

👍 seems to be working correctly for me too with the 4.2.0.beta1

@zenspider

This comment has been minimized.

Show comment
Hide comment
@zenspider

zenspider Sep 8, 2014

I FOUND IT! Filing an issue w/ details.

I FOUND IT! Filing an issue w/ details.

@lucasmazza

This comment has been minimized.

Show comment
Hide comment
@lucasmazza

lucasmazza Sep 8, 2014

Contributor

Thanks to @zenspider and minitest-bisect I've managed to fix most of the order dependent tests in Devise, except for the one related to storing custom i18n translations during the test using this helper, and I don't know if this snippet is enough to deal with this with the latest versions of i18n. @carlosantoniodasilva, any idea of anything that we might be overlooking here? This isn't strictly related to the 4.2 compatibility but would be awesome to snag it while we are here.

Contributor

lucasmazza commented Sep 8, 2014

Thanks to @zenspider and minitest-bisect I've managed to fix most of the order dependent tests in Devise, except for the one related to storing custom i18n translations during the test using this helper, and I don't know if this snippet is enough to deal with this with the latest versions of i18n. @carlosantoniodasilva, any idea of anything that we might be overlooking here? This isn't strictly related to the 4.2 compatibility but would be awesome to snag it while we are here.

@zenspider

This comment has been minimized.

Show comment
Hide comment
@zenspider

zenspider Sep 8, 2014

A total aside, that inner begin/end doesn't need to be there. It is implied by the def. Same goes with rescues and associated else's.=

A total aside, that inner begin/end doesn't need to be there. It is implied by the def. Same goes with rescues and associated else's.=

lucasmazza added some commits Sep 11, 2014

Ensure that the I18n backend is always initialized when we store cust…
…om translations.

Depending on the test order, there might a moment when a test reloads the I18n
backend and another tries to store a translation, but since the backend wasn't
re-initialized the custom translations would be overriden when i18n loads the
translations from the en.yml file.
Remove test ordering setup
We can now run the test suite on random order.
@lucasmazza

This comment has been minimized.

Show comment
Hide comment
@lucasmazza

lucasmazza Sep 11, 2014

Contributor

Fixed a couple more tests today and I couldn't find any other failing seeds after rerunning the tests 100 times. ✌️

Contributor

lucasmazza commented Sep 11, 2014

Fixed a couple more tests today and I couldn't find any other failing seeds after rerunning the tests 100 times. ✌️

@zenspider

This comment has been minimized.

Show comment
Hide comment
@zenspider

zenspider Sep 11, 2014

Fan-fucking-tastic!

Fan-fucking-tastic!

@deepj

This comment has been minimized.

Show comment
Hide comment
@deepj

deepj Sep 30, 2014

I've been trying to migrate an application (which I haven't developed before) to Rails 4.2 (mainly due to ActiveJob) and I'm using this branch due to incompatibility with Rails 4.2 instead of Devise 3.3.0 (where everything works fine) . Unfortunately, I haven't worked with Devise for a while (I was stuck in 2.x). So I'm not sure how to deal with this issue.

I'm getting the following issue when I run it in production environment (no problem in development or test environment).

 app/controllers/application_controller.rb:6:in `alias_method': undefined method `current_api_user' for class `ApplicationController' (NameError)

My app/controllers/application_controller.rb looks something like

class ApplicationController < ActionController::Base
  # Prevent CSRF attacks by raising an exception.
  # For APIs, you may want to use :null_session instead.
  protect_from_forgery with: :exception

  alias_method :current_user, :current_api_user
  alias_method :user_signed_in?, :api_user_signed_in?
end

In config/routes.rb

namespace :api, defaults: { format: :json } do
  devise_for :users, skip: :registrations, controllers: { sessions: 'api/auth', omniauth_callbacks: 'api/omniauth_callbacks' }

  devise_scope :api_user do
    post :logout, to: 'auth#destroy', as: :destroy_user_session
    post :login,  to: 'auth#create',  as: :create_user_session

    get 'auth/facebook/callback', to: 'api/omniauth_callbacks#facebook'
    get 'auth/google/callback',   to: 'api/omniauth_callbacks#google'
  end
end

I'm not sure if the customized Devise configuration is used properly. The same problem I'm getting with Rails 4.1.6 when I use this branch. There is no problem with Devise 3.3.0. I use Ruby 2.1.3.

deepj commented Sep 30, 2014

I've been trying to migrate an application (which I haven't developed before) to Rails 4.2 (mainly due to ActiveJob) and I'm using this branch due to incompatibility with Rails 4.2 instead of Devise 3.3.0 (where everything works fine) . Unfortunately, I haven't worked with Devise for a while (I was stuck in 2.x). So I'm not sure how to deal with this issue.

I'm getting the following issue when I run it in production environment (no problem in development or test environment).

 app/controllers/application_controller.rb:6:in `alias_method': undefined method `current_api_user' for class `ApplicationController' (NameError)

My app/controllers/application_controller.rb looks something like

class ApplicationController < ActionController::Base
  # Prevent CSRF attacks by raising an exception.
  # For APIs, you may want to use :null_session instead.
  protect_from_forgery with: :exception

  alias_method :current_user, :current_api_user
  alias_method :user_signed_in?, :api_user_signed_in?
end

In config/routes.rb

namespace :api, defaults: { format: :json } do
  devise_for :users, skip: :registrations, controllers: { sessions: 'api/auth', omniauth_callbacks: 'api/omniauth_callbacks' }

  devise_scope :api_user do
    post :logout, to: 'auth#destroy', as: :destroy_user_session
    post :login,  to: 'auth#create',  as: :create_user_session

    get 'auth/facebook/callback', to: 'api/omniauth_callbacks#facebook'
    get 'auth/google/callback',   to: 'api/omniauth_callbacks#google'
  end
end

I'm not sure if the customized Devise configuration is used properly. The same problem I'm getting with Rails 4.1.6 when I use this branch. There is no problem with Devise 3.3.0. I use Ruby 2.1.3.

@zenspider

This comment has been minimized.

Show comment
Hide comment
@zenspider

zenspider Sep 30, 2014

@deepj please don't piggy back on other issues. File a new issue.

@deepj please don't piggy back on other issues. File a new issue.

@deepj

This comment has been minimized.

Show comment
Hide comment
@deepj

deepj Sep 30, 2014

@zenspider I don't think that belongs to somewhere else. I just describe my issues and experiences related to this pull request.

deepj commented Sep 30, 2014

@zenspider I don't think that belongs to somewhere else. I just describe my issues and experiences related to this pull request.

@lucasmazza

This comment has been minimized.

Show comment
Hide comment
@lucasmazza

lucasmazza Oct 2, 2014

Contributor

@deepj can you replicate this in a fresh app and push it to GitHub? Then I take a look and check what is really going wrong with this branch.

Contributor

lucasmazza commented Oct 2, 2014

@deepj can you replicate this in a fresh app and push it to GitHub? Then I take a look and check what is really going wrong with this branch.

@deepj

This comment has been minimized.

Show comment
Hide comment
@deepj

deepj Oct 2, 2014

@lucasmazza Here is the repository for replication of the problem

https://github.com/deepj/devise-rails42

Just run it like bundle exec rails s -e production

deepj commented Oct 2, 2014

@lucasmazza Here is the repository for replication of the problem

https://github.com/deepj/devise-rails42

Just run it like bundle exec rails s -e production

@lucasmazza

This comment has been minimized.

Show comment
Hide comment
@lucasmazza

lucasmazza Oct 2, 2014

Contributor

@deepj thanks! I took a look on your app and seems to me that, when the Rails config eager_load is true, the application will load the ApplicationController before loading the routes file, so in that moment the Devise helpers won't be available. I'll recheck this loading order in different Rails versions.

Contributor

lucasmazza commented Oct 2, 2014

@deepj thanks! I took a look on your app and seems to me that, when the Rails config eager_load is true, the application will load the ApplicationController before loading the routes file, so in that moment the Devise helpers won't be available. I'll recheck this loading order in different Rails versions.

@lucasmazza

This comment has been minimized.

Show comment
Hide comment
@lucasmazza

lucasmazza Oct 2, 2014

Contributor

Dug some more, the culprit is #3195, not the 4.2 compatibility code.

Contributor

lucasmazza commented Oct 2, 2014

Dug some more, the culprit is #3195, not the 4.2 compatibility code.

@lucasmazza

This comment has been minimized.

Show comment
Hide comment
@lucasmazza

lucasmazza Oct 3, 2014

Contributor

@deepj can you test this branch one more time? ❤️ 💚 💙 💛 💜

Contributor

lucasmazza commented Oct 3, 2014

@deepj can you test this branch one more time? ❤️ 💚 💙 💛 💜

@deepj

This comment has been minimized.

Show comment
Hide comment
@deepj

deepj Oct 3, 2014

@lucasmazza It seems everything is green! 👍

deepj commented Oct 3, 2014

@lucasmazza It seems everything is green! 👍

@lucasmazza

This comment has been minimized.

Show comment
Hide comment
@lucasmazza

lucasmazza Oct 3, 2014

Contributor

Awesome!

Contributor

lucasmazza commented Oct 3, 2014

Awesome!

lucasmazza added a commit that referenced this pull request Oct 3, 2014

@lucasmazza lucasmazza merged commit 8e5c098 into master Oct 3, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
@rubytastic

This comment has been minimized.

Show comment
Hide comment
@rubytastic

rubytastic Nov 28, 2014

Having an initialiser for splitting up routes like so:

class ActionDispatch::Routing::Mapper
def draw(routes_name)
instance_eval(File.read(Rails.root.join("config/routes#{@scope[:path]}", "#{routes_name}.rb")))
end
end

Still breaks rails 4.2beta2

Having an initialiser for splitting up routes like so:

class ActionDispatch::Routing::Mapper
def draw(routes_name)
instance_eval(File.read(Rails.root.join("config/routes#{@scope[:path]}", "#{routes_name}.rb")))
end
end

Still breaks rails 4.2beta2

@mmahalwy

This comment has been minimized.

Show comment
Hide comment
@mmahalwy

mmahalwy Dec 6, 2014

undefined local variable or method `mimes_for_respond_to' for DeviseController:Class

Still happening in 4.2.0.rc1

mmahalwy commented Dec 6, 2014

undefined local variable or method `mimes_for_respond_to' for DeviseController:Class

Still happening in 4.2.0.rc1

@promiseverma

This comment has been minimized.

Show comment
Hide comment
@promiseverma

promiseverma Jan 23, 2015

replace
gem 'devise'
to
gem 'devise', :git => 'https://github.com/plataformatec/devise.git'

replace
gem 'devise'
to
gem 'devise', :git => 'https://github.com/plataformatec/devise.git'

@joshdance

This comment has been minimized.

Show comment
Hide comment

I was still having errors. This SO answer fixed it for me - http://stackoverflow.com/questions/27611947/devise-raises-error-with-rails-4-2-upgrade

@amritdeep

This comment has been minimized.

Show comment
Hide comment
@amritdeep

amritdeep Mar 10, 2016

Use devise 3.4.1.

Use devise 3.4.1.

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