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

Fix Ruby 2.7 deprecation warning on Mounter#remove? #2644

Merged
merged 1 commit into from
Dec 30, 2022

Conversation

kaoru
Copy link

@kaoru kaoru commented Dec 21, 2022

We're currently working on our Ruby 2.7 to 3.0 upgrade and we're testing with deprecation warnings enabled on Ruby 2.7. Most of the warnings we've seen have been related to keyword arguments, but there was a different one coming from Carrierwave:

.../carrierwave-1.3.2/lib/carrierwave/mounter.rb:113: warning: deprecated Object#=~ is called on TrueClass; it always returns nil

This PR aims to silence that warning with no change in behaviour on any version of Ruby.

Unfortunately I wasn't able to get the tests running in my M1 Mac development environment. It looks like the project has GitHub actions set up so hopefully that will work ok 😄

Sorry we're still on 1.x 😰 Many thanks to all the maintainers 🙏

@@ -110,7 +110,7 @@ def blank?
end

def remove?
remove.present? && remove !~ /\A0|false$\z/
remove.present? && remove.to_s !~ /\A0|false$\z/
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the PR, could you make this same as the fix in the master branch?
https://github.com/carrierwaveuploader/carrierwave/blob/master/lib/carrierwave/mounter.rb#L115

Copy link
Author

Choose a reason for hiding this comment

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

Done at 711d612 😄 Thanks!

On Ruby 2.7, with deprecation warnings enabled, the Mounter#remove? code
which handles string arguments to remove that we want to be treated as
false-y ("0" and "false") was throwing a deprecation warning:

"warning: deprecated Object#=~ is called on TrueClass; it always returns
nil"

This change fixes that warning by checking if the value is TrueClass
before running the regular expression.

Note that this does not change any behaviour since the presence check
was already handling the case that a false Boolean was passed, and the
case that a true Boolean was passed was already correctly not matching
the regular expression albeit with the deprecation warning.

This fix is the same as the one on the master branch commit
9a37fc9 by mshibuya.
@mshibuya mshibuya merged commit 1885df6 into carrierwaveuploader:1.x-stable Dec 30, 2022
@mshibuya
Copy link
Member

Thank you!

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