-
Notifications
You must be signed in to change notification settings - Fork 156
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
Import ActionController::MissingRenderer from Rails #245
base: main
Are you sure you want to change the base?
Conversation
@zzak I can get this into responders and released, but we'll have to update the Rails side of things a bit apparently... we might also have to detect here if the constant is not already defined or something (suppressing possible warnings), I think? |
@carlosantoniodasilva Thanks for the ping, yeah I'm not sure but maybe we can do I have to fix the tests (and probably rebase) upstream too, but I think the right thing to do is do the check on responders side. |
4179e62
to
80a5470
Compare
80a5470
to
bdf7d11
Compare
@carlosantoniodasilva Finally got around to this again, and after fixing the deprecation on Rails side, I think the best path forward is to rename the constant inside this gem. If someone is using an unpatched version of the
That was before I renamed it internally, I don't think is a problem, if someone is catching this error manually they will also get the warning. The solution will be to update the There is a gap however, if someone skips the version of Rails which emits the deprecation warning AND has an unpatched responders gem their code may break -- but that is an acceptable trade-off and people should perform upgrades gradually without skipping. |
Happened upon this exception when patching this file in rails/rails#48327, the documentation references a method which was moved to this gem, and nothing else in Rails uses it internally.
I think we can move it here and deprecate it from Rails, which I've started working on in rails/rails#48328