-
Notifications
You must be signed in to change notification settings - Fork 485
Remove respond_to? check #2202
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
Remove respond_to? check #2202
Conversation
f070b12 to
14c5cf1
Compare
af2eadc to
691f55c
Compare
691f55c to
450dfd1
Compare
992be13 to
ffb02e3
Compare
| def __vc_request | ||
| @__vc_request ||= controller.request if controller.respond_to?(:request) | ||
| @__vc_request ||= controller.request | ||
| rescue NoMethodError |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this change goes against this project's code style? 🤔
|
@tiagomenegaz Firstly, thank you for this work 😄 |
No problem. Any recommendations for that? @reeganviljoen |
|
I feel like there's context I'm missing. What's the reasoning behind this change? |
|
@BlakeWilliams I looked at some of these with @tiagomenegaz. We were looking to see if we could get rid of these checks wholesale as many were implicitly for older versions of Rails. I'll review this! |
|
Closing in favor of #2219, added you as co-author! |
Thanks for letting me know @joelhawksley |
What are you trying to accomplish?
This PR removes
respond_to?from the following files:app/controllers/concerns/view_component/preview_actions.rblib/view_component/base.rblib/view_component/collection.rblib/view_component/slotable.rblib/view_component/slotable_default.rblib/view_component/translatable.rbtest/sandbox/test/rendering_test.rbtest/sandbox/test/slotable_test.rbview_component.gemspecMost of the changes were done by refactoring
respond_to?toclass.method_definedwhich is an alternative to that method. Please feel free to discuss these changes.What approach did you choose and why?
Some checks aren't necessary because we don't support discontinued Ruby versions and the code can be written without
respond_to?.Anything you want to highlight for special attention from reviewers?
I noticed that some
respond_to?are being used to supportruby2_keywords. I'm assuming it exists because we want to support old ruby versions. However, based on the gemfile -ruby_version = (ENV["RUBY_VERSION"] || "~> 3.4").to_s- we don't support Ruby 2 anymore. I'll address these keywords in a different PR.