[Fix #3566] Add new Metrics/BlockLength cop.
#3587
Conversation
| private | ||
|
|
||
| def message(length, max_length) | ||
| format('Block has too many lines. [%d/%d]', length, max_length) |
There was a problem hiding this comment.
I'd extract this string to constant.
There was a problem hiding this comment.
This uses a consistent style with MethodLength. Also this line is refactored into another file (and put in a constant) in the next commit. Let me know if you still want it done.
|
The new cop looks good. Just drop those trailing |
This cop checks there aren't too many lines in blocks, much like `Metric/MethodLength` checks for too many lines in methods. Also removed the splatted additional args from CodeLength#check_code_length, I don't think they've been necessary since the old `check` method was refactored into that method.
The `CodeLength` inclusion, `#message` method, and `#code_length` are shared between both cops so have been moved into a mixin included by both. Also added two test lines that ensure the messages are being set properly.
I'd prefer a default of 10 - same as Other than that, I love the new cop! |
|
I started off thinking 10 was a good idea too until I saw how many places it failed. 😋 Admittedly excluding it on specs and rake things helped, but there's still quite a lot. I imagine every Rails application's Perhaps we should put this in a new issue so more people see it and can voice opinions? I imagine not many will in here. |
This cop checks there aren't too many lines in blocks, much like
Metric/MethodLengthchecks for too many lines in methods.By default the max lines has been set to 25 which seems a decent starting point as the Rubocop library passes without having to change our config file (but only due to the default excluded files).
The cop is excluded from checking the
Rakefile, all.rakefiles, and all Ruby files inspecby default. All of these are pretty much intended to be large blocks, the length of classes or modules, and will easily breach a lowish limit.I've left this as 2 separate commits, the first of which adds the cop, and the second of which refactors shared parts into a mixin. I'm not sure if there's a nicer way to make each cop's message work without having to have a method in each cop to pass its label through, like this.
Before submitting the PR make sure the following are checked:
[Fix #issue-number](if the related issue exists).master(if not - rebase it).and description in grammatically correct, complete sentences.