-
Notifications
You must be signed in to change notification settings - Fork 12
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 unused rack classes #10
Conversation
I don't think we can merge this as it will break compatibility. However, I'll check tomorrow. |
If merged and released in a new major version of the |
Yes, that would be totally fine, however my suggestion is, there is no hurry to do this, and the sooner we do it, the more pain we will cause. So, IMHO, let's wait until Rack 3.0 and Rails 7.1 is well and truly out the door, maybe even Rails 8.0 - Rack 2.x will likely be deprecated around that time, then this change will be painless. Unless there is some value in doing it sooner, I'd vote we wait to avoid creating potential pain for users. |
Totally agree with that. |
If we are going to keep these for a while, should the autoload logic be copied to rackup instead? |
I think that’s reasonable. |
0d1c9e8
to
62e8e82
Compare
These are no longer autoloaded so they can be removed. See: rack/rack@fd7e0ad
62e8e82
to
5e510d9
Compare
I've added both Puma and Falcon to the external test suite, and everything is passing, so I'm good to merge this now. Tangentially related, I'm happy that we gave this time to percolate through the system, so thanks for your patience. |
Released in v2.2.0. |
These are no longer autoloaded so they can be removed.
See:
rack/rack@fd7e0ad
rack/rack#2113 (comment)