redis-rb / redis-client

Simple low level client for Redis 6+
MIT License
125 stars 60 forks source link

Use more Rubocop defaults #62

Closed jcraigk closed 1 year ago

jcraigk commented 1 year ago

There are many instances of Enabled: false in https://github.com/redis-rb/redis-client/blob/master/.rubocop.yml. Why even use Rubocop if all of the most important cops are disabled?

byroot commented 1 year ago

Because plenty of rubocop rules are either stupid or are unable to deal with some corner cases. So whenever rubocop suggest something stupid, I disable that rule.

jcraigk commented 1 year ago

I'm interpreting your use of the word "stupid" to mean "inconvenient for the maintainer". As producers and consumers of Ruby, I and many other developers highly value the following cops (not an exhaustive list):

Lint/EmptyBlock
Lint/RescueException
Metrics/AbcSize:
Metrics/BlockLength
Metrics/ClassLength
Metrics/CyclomaticComplexity
Metrics/MethodLength
Metrics/PerceivedComplexity

When I took a look at https://github.com/redis-rb/redis-client/blob/master/lib/redis_client/config.rb#L156 to debug an issue, I was immediately confronted by a long complex method that, according to Rubocop rules, should be broken into smaller well-named methods. This improves readability, reducing the amount of time one would need to spend reasoning about the code. It also improves maintainability so that other developers can refactor specific methods and review PRs more readily.

I urge you to reconsider your opinions on these cops. Thanks for reading.

casperisfine commented 1 year ago

I'm interpreting your use of the word "stupid" to mean "inconvenient for the maintainer".

No I really mean "stupid".

I urge you to reconsider your opinions on these cops.

Final no.