-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Bring ABC size down to 16 #7453
Conversation
@@ -429,6 +429,7 @@ def compile_ellipsis | |||
[0..Float::INFINITY, 'true'] | |||
end | |||
|
|||
# rubocop:disable Metrics/AbcSize |
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.
Couldn't find any way to refactor this method that didn't make it worse. Pretty sure this is as good as it gets with the current design, but open to suggestions.
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.
Challenge accepted 😉 #7457
🙇 |
@condition += 1 | ||
end | ||
|
||
def else_branch?(node) | ||
%i[case if].include?(node.type) && |
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.
@jonas054 I suspect we could have a subtle bug here. We check for #else_branch?
on CONDITION_NODES
, which comes from CyclomaticComplexity::COUNTED_NODES
, but that list does not include case
:
COUNTED_NODES = %i[if while until for
rescue when and or].freeze
so meta 😆 |
I wanted to cement the excellent work done by @buehmann in #7450 by bringing the bar down to 16. That way we can't have new offenses creeping in without notice.
Before submitting the PR make sure the following are checked:
[Fix #issue-number]
(if the related issue exists).master
(if not - rebase it).bundle exec rake default
. It executes all tests and RuboCop for itself, and generates the documentation.