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

Conversation

bendangelo
Copy link
Contributor

Updated argument to color: true. This stops the rails 7.2 dep warning.

@bendangelo bendangelo requested a review from solnic as a code owner June 27, 2024 15:19
@flash-gordon
Copy link
Member

This should check for the rails version then

@bendangelo
Copy link
Contributor Author

Ok done.

Comment on lines +36 to +41
def color_option
if Gem::Version.new(Rails::VERSION::STRING) >= Gem::Version.new('7.1')
{ color: true }
else
true
end
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.

flash-gordon added a commit that referenced this pull request Jul 1, 2024
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.

2 participants