-
Notifications
You must be signed in to change notification settings - Fork 341
Fix Ruby 2.4 deprecations #216
base: master
Are you sure you want to change the base?
Fix Ruby 2.4 deprecations #216
Conversation
lib/devise_security_extension/models/password_archivable.rb:20: warning: constant ::Fixnum is deprecated
This requires Rails 4.2.8+ to get the correct version of the json gem
Use assert_nil if expecting nil from /devise_security_extension/test/test_paranoid_verification.rb:64:in `block in <class:TestParanoidVerification>'. This will fail in MT6.
| s.required_ruby_version = '>= 2.1.0' | ||
|
|
||
| s.add_runtime_dependency 'railties', '>= 3.2.6', '< 6.0' | ||
| if RUBY_VERSION >= '2.4' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is pre Rails 4.2.8 compatibility important? If so, is this the way you want to support it? Ruby 2.4+ requires Rails 4.2.8+ because of changes in the json gem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, its easy enough to upgrade to 4.2.[8, 9, 10] we should not need to support previous versions
|
This successfully removes the warnings for me on Ruby 2.4.1 👍 |
|
Is there anything stopping merging this branch? This warning is driving me crazy. |
|
Someone with merge permissions |
| user = User.new(paranoid_verification_code: 'abcde') | ||
| user.verify_code('wrong') | ||
| assert_equal(nil, user.paranoid_verified_at) | ||
| assert_nil(user.paranoid_verified_at) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rip
|
Please merge |
|
Is this gem still actively maintained? |
|
Unfortunately it doesn't appear so. A couple of us have made a fork called devise-security |
| # validate is the password used in the past | ||
| def password_archive_included? | ||
| unless deny_old_passwords.is_a? Fixnum | ||
| unless deny_old_passwords.is_a? 1.class |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not use Integer here? Even in Ruby < 2.4:
1.class #=> Fixnum
1.is_a? Integer #=> trueThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The maintainers of this repo seemed to have disappeared, so a few of us are maintaining a fork at https://github.com/devise-security/devise-security. Feel free to make a PR with these changes over there.
|
|
||
| def expired_password_after_numeric? | ||
| return @_numeric if defined?(@_numeric) | ||
| @_numeric ||= self.expire_password_after.is_a?(1.class) || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
||= is redundant if you're using a guard
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point
| end | ||
|
|
||
| def expired_password_after_numeric? | ||
| return @_numeric if defined?(@_numeric) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it actually a good idea to memoize this? Could there be a scenario where this gets checked and then the column is made to be nil?
expired_password_after_numeric? #=> true
self.expire_password_after = nil
expired_password_after_numeric? #=> trueThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately I don't know the code well enough to say.
|
Any reason to not merge this? |
|
@tibbon |
lib/devise_security_extension/models/password_archivable.rb:20: warning: constant ::Fixnum is deprecated
I'm upgrading my app to Ruby 2.4 and getting these deprecations everywhere.