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

Move respond_with to the responders gem #16526

Merged
merged 3 commits into from
Aug 17, 2014
Merged

Move respond_with to the responders gem #16526

merged 3 commits into from
Aug 17, 2014

Conversation

josevalim
Copy link
Contributor

respond_with (and consequently the class-level respond_to)
are being removed from Rails. Instead of moving it to a 3rd
library, the functionality will be moved to responders gem
(at github.com/plataformatec/responders) which already provides
some responders extensions.

@seuros
Copy link
Member

seuros commented Aug 16, 2014

caching_with_rails.md
contributing_to_ruby_on_rails.md

@josevalim
Copy link
Contributor Author

@seuros good catch, thanks!

@@ -21,7 +19,7 @@ def zero
def create
ActiveRecord::LogSubscriber.runtime += 100
project = Project.last
respond_with(project, location: url_for(action: :show))
redirect_to "/"
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't be ?

redirect_to url_for(action: :show, id: project.id)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are not testing the redirect location, it can be anything.

@robin850
Copy link
Member

Nice work ! As for the responders gem, once you're sure about the version you'll pick for the next release, could you please add it to the generated Gemfile in new applications ? Also I guess you can mention this removal plus the gem name in the upgrading guide. Thanks ! :-)

@robin850 robin850 added this to the 4.2.0 milestone Aug 17, 2014
@carlosantoniodasilva
Copy link
Member

👍

@robin850 I don't think we'll default to adding the gem in the Gemfile for new apps.

José Valim added 3 commits August 17, 2014 13:20
respond_with (and consequently the class-level respond_to)
are being removed from Rails. Instead of moving it to a 3rd
library, the functionality will be moved to responders gem
(at github.com/plataformatec/responders) which already provides
some responders extensions.
josevalim pushed a commit that referenced this pull request Aug 17, 2014
Move respond_with to the responders gem
@josevalim josevalim merged commit bf5b696 into master Aug 17, 2014
@josevalim josevalim deleted the jv-no-responders branch August 17, 2014 17:21
@guilhermesimoes
Copy link

What was the rationale for removing this feature? It saw little use?

@matthewd
Copy link
Member

#12136 (comment)

@guilhermesimoes
Copy link

Thanks! Good to know.

@runwaldarshu
Copy link

I am wondering if I add responders gem into gemspec why will it still need to be added in Gemfile where gemspec is included in Gemfile.
I am seeing failures while doing that.

@jedschneider
Copy link

Is rails not using semantic versioning? This should not have been included in a minor release without providing backwards support for 4.x apps that use responders. A deprecation warning and request to move would have been OK, but breaking changes should have been done at 5.0.

@rafaelfranca
Copy link
Member

Is rails not using semantic versioning?

http://guides.rubyonrails.org/maintenance_policy.html

This should not have been included in a minor release without providing backwards support for 4.x apps that use responders.

http://guides.rubyonrails.org/4_2_release_notes.html#respond-with-class-level-respond-to

@chancancode
Copy link
Member

@runwaldarshu when adding something to the gemspec, it will add that library to your gem's dependency graph, so that when you gem install your-gem, rubygems would know that it needs to install your-dependency too. However, it does not automatically require your-dependency for you.

On the other hand, the Gemfile in your-library (as opposed to your Rails app's Gemfile) is only used for development purpose, and does not affect the dependency graph of your published gem (your-library). However, adding something to the Gemfile also, by default, requires it automatically (unless you specified require: false).

So, if you are packaging a gem/engine that uses responsers, your set up should probably look like this:

# your_engine/your_engine.gemspec

  ...
  s.add_dependency "responsers", "..."
  ...
# your_engine/Gemfile

gemspec
# your_engine/lib/your_engine.rb
require 'your_engine/version'
...
require 'responders'
...

@jedschneider
Copy link

@rafaelfranca thanks for the clarification. Working with a student that is having a hard time getting responders loaded in her project. Since my last comment I upgraded an app to 4.2 without issue, but devise was included so it was already complaining that I needed the responders gem for that dependency to work. For me it was seamless, but apparently not for everyone.

@rafaelfranca
Copy link
Member

@jedschneider the version of Devise compatible with Rails 4.2 already include responders gem as dependecy https://github.com/plataformatec/devise/blob/v3.4.1/devise.gemspec#L28

@jedschneider
Copy link

@rafaelfranca FWIW had to bundle update devise as well to pick up that change (and it did work). But I'd probably rather leave the dependency fully qualified in my Gemfile anyway, since I'm using responders directly in my controllers. Thanks again.

@jedschneider
Copy link

@rafaelfranca worked through the student's issue. I think the confusion was associated with the installation of the responders gem, and the bootstrapping that it does with the generator, vs what you need to do directly in rails (just require). I'm not sure thats worth a note or not.

@rafaelfranca
Copy link
Member

@jedschneider yeah, installation of responders seems to confuse people. Maybe we should make explicit that the install process is optional at responders README.

@lacostenycoder
Copy link

I've read through this but don't quite understand "why" the responders have been removed from rails. Is there a better way of doing this in rails 4.2 ? I should have checked here fires. Never mind.
http://stackoverflow.com/questions/25998437/why-is-respond-with-being-removed-from-rails-4-2-into-its-own-gem

@rafaelfranca
Copy link
Member

@lacostenycoder with variants and things like jbuilder this feature becomes out of date. The idea of variants would not work with it so we chose to just remove from the core. Pay attention that the feature is still supported, just outside of the default Rails stack.

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

Successfully merging this pull request may close these issues.

None yet