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

Support Rails 7.0 #97

Merged
merged 1 commit into from
Nov 13, 2024
Merged

Conversation

ofedoren
Copy link
Contributor

@ofedoren ofedoren commented Nov 8, 2024

No description provided.

@ofedoren ofedoren force-pushed the support-rails-7 branch 3 times, most recently from 89ac933 to f063521 Compare November 8, 2024 15:17
@@ -7,8 +7,9 @@ module RealmTheme
Realm.class_eval do
_validators.delete(:realm_type)
_validate_callbacks.each do |callback|
if callback.raw_filter.respond_to? :attributes
callback.raw_filter.attributes.delete :realm_type
filter_method = callback.respond_to?(:raw_filter) ? :raw_filter : :filter
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the difference here? Is this something for compatibility with old/new versions that we can drop at some point?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeap, that's just for the compatibility. Although, we tend to require Foreman version in engines, I'd say we don't even need callback.respond_to?(:raw_filter) compatibility fix, just bump required version to >= 3.14 and use filter right away.

raw_filter was removed in Rails 7, filter seems to be the same:

Copy link
Member

Choose a reason for hiding this comment

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

6.1 aliases raw_filter to filter, so we can use that already and don't have to do anything special here? Or am I reading the links wrong? 6.1 has been a requirement for a long time.

Copy link
Contributor

Choose a reason for hiding this comment

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

Reading the code, I see for 6.1:

        def filter; @key; end
        def raw_filter; @filter; end

And 7.0:

        attr_reader :chain_config, :filter

So 6.1 has filter read @key while 7.0 reads @filter. In other words, filter changes behavior between the versions.

Copy link
Member

Choose a reason for hiding this comment

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

🤯
Did I mention how much I like Rails?

Copy link
Contributor

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

I'm good with the compatibility approach, though I'd appreciate a code comment above it mentioning it's for Rails 6.1 & 7.0 compatibility so if we forget and later see it that we easily know.

@evgeni what do you prefer? hard cut or compatibility?

@evgeni
Copy link
Member

evgeni commented Nov 12, 2024

I'm good with the compatibility approach, though I'd appreciate a code comment above it mentioning it's for Rails 6.1 & 7.0 compatibility so if we forget and later see it that we easily know.

@evgeni what do you prefer? hard cut or compatibility?

The filter method thing is the only one, right? Then I'd go with compat as it doesn't seem super complex.

@evgeni evgeni merged commit 343c1b9 into RedHatSatellite:develop Nov 13, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants