Improve rubocop settings #131
Conversation
|
I'd prefer to leave nothing to |
|
@tarcieri What do you mean by "leave leave nothing to .rubocop_todo.yml" |
|
@gssbzn I mean either fix the problems or add appropriate overrides to |
|
@tarcieri I removed the reference to the todo file Let me know if this is fine or if there's another thing I should look at |
| end | ||
| rescue FFI::NotFoundError | ||
| def initialize(_opslimit, _memlimit, _digest_size = 64) | ||
| raise NotImplementedError, "scrypt not implemented in this version of libsodium" |
There was a problem hiding this comment.
I guess you're declaring bankruptcy on older versions of libsodium that don't implement scrypt here?
There was a problem hiding this comment.
This was the harder part to try to fix
I was checking the ffi gem, and the Sodium module and my conclusion was that the rescue was there for the sodium_function calls.
But at the same time I checked for other classes that use the sodium_function and they don't rescue in case a definition with fii fails.
Is this rescue only for scrypt case? Should the scrypt function only be exposed through RbNaCl::PasswordHash? If that's the case maybe only a require of this class in this module and handle the rescue there.
There was a problem hiding this comment.
Or I can update the README to reflect the minimum libsodium
There was a problem hiding this comment.
Yeah, looks like scrypt was added prior to 1.0. I think it's fine to note in the README that the minimum version is 1.0.0.
|
This looks fine with one noted caveat |
- Apply recent rubocop autocorrections to code base - Small code improvements to comply to rubocop
.rubocop_todo.ymlfile and reference it on settings file