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

Branch out 4-stable from v4.9.2 with Rails 7.1 compatibility #5639

Closed
wants to merge 16 commits into from

Conversation

carlosantoniodasilva
Copy link
Member

@carlosantoniodasilva carlosantoniodasilva commented Oct 10, 2023

This branches off the 4.9.2 tag instead of main, to add Rails 7.1 compatibility and fix the build in the process.

There's been some changes in main that I am not yet sure about releasing as 4.x, and I also want to drop support for older versions of Ruby/Rails soon (#5600) because it's getting harder and harder to keep those builds green, so I'm going to target main as our 5.x going forward, which means 4-stable will be base for the upcoming release.

Note: While I may cherry-pick some things in here back to main, I'm opening this as a PR against main more for visibility and reference than anything else. This should make it easier to point people to, and have a discussion about, any issues that may arise while we don't release.

There's a number of deprecation warnings to work through related to
mocha updates in v2+, we'll get through those on a separate change.
https://github.com/freerange/mocha/blob/main/RELEASE.md#200

The main issue is with Minitest, fixed in v2.1:
https://github.com/freerange/mocha/blob/main/RELEASE.md#210

Also run `bundle update` on the main Gemfile to update all dependencies
there to latest.
Use a dedicated ActiveSupport::Deprecation
…t_deprecation

Fixed missing migration to dedicated deprecator
We still support super old versions, yes, and it doesn't like `ensure`
without a `begin..end` unfortunately.

I plan to remove this support soon, but for now I don't want to stop
supporting it yet.
There's some incompatibility issue with loofah there since it uses an
older version of nokogiri, so I'm locking it on those older versions to
try to get a green build again there.
Trying to get the build fully green for now.
There was a change introduced in Rails 7.1 that causes all public
actions of non-abstract controllers to become action methods, even if
they happen to match the name of an internal method defined by abstract
`ActionController::Base` and such, which is the case with `_prefixes`.

This change was intentional, it allows for example to have an action
called `status`, which is an internal method, and that is properly
managed as an action method now. However, it broke Devise due to
overriding `_prefixes`, which is a public method of Action Controller.

To fix, we are simply ensuring we keep `_prefixes` as an internal method
rather than action method, which matches previous behavior for this
particular method/implementation in Devise.

Ref: rails/rails#48699
@carlosantoniodasilva carlosantoniodasilva self-assigned this Oct 10, 2023
app/controllers/devise_controller.rb Outdated Show resolved Hide resolved
lib/devise/models/authenticatable.rb Show resolved Hide resolved
Copy link
Member Author

@carlosantoniodasilva carlosantoniodasilva left a comment

Choose a reason for hiding this comment

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

Thanks @eebs!

lib/devise/models/authenticatable.rb Show resolved Hide resolved
app/controllers/devise_controller.rb Outdated Show resolved Hide resolved
@excid3
Copy link
Contributor

excid3 commented Oct 11, 2023

Tested against our applications and everything works well. 🎉

@carlosantoniodasilva
Copy link
Member Author

Thanks @excid3!

I have just released v4.9.3. I'll port some of these commits to main and keep 4-stable as the v4 branch going forward.

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

Successfully merging this pull request may close these issues.

None yet

4 participants