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

Fixes Rails 7.2 dep warning #426

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions lib/rom/sql/extensions/rails_log_subscriber.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,31 @@
binds = payload[:binds].to_a.inspect if payload[:binds]

if odd?
name = color(name, :cyan, true)
sql = color(sql, nil, true)
name = color(name, :cyan, color_option)
sql = color(sql, nil, color_option)
else
name = color(name, :magenta, true)
name = color(name, :magenta, color_option)
end

debug " #{name} #{sql} #{binds}"
end

attr_reader :odd_or_even
private :odd_or_even

def odd?
@odd_or_even = !odd_or_even
end

private

def color_option
if Gem::Version.new(Rails::VERSION::STRING) >= Gem::Version.new('7.1')

Check failure on line 37 in lib/rom/sql/extensions/rails_log_subscriber.rb

View workflow job for this annotation

GitHub Actions / Style

Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
{ color: true }

Check failure on line 38 in lib/rom/sql/extensions/rails_log_subscriber.rb

View workflow job for this annotation

GitHub Actions / Layout

Layout/SpaceInsideHashLiteralBraces: Space inside { detected.

Check failure on line 38 in lib/rom/sql/extensions/rails_log_subscriber.rb

View workflow job for this annotation

GitHub Actions / Layout

Layout/SpaceInsideHashLiteralBraces: Space inside } detected.
else
true
end
Comment on lines +36 to +41
Copy link
Member

Choose a reason for hiding this comment

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

I have two suggestions

  1. You can assign it to a constant COLOR_OPTION = if ... will work just as fine but with only 1 check rather than a check per every call
  2. You use Rails constant but the plugin depends on active-support, you should check its version instead. For instance, I have a project where active-support is present but rails isn't.

end
end
end
end
Expand Down
Loading