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
[Fix #7396]: Display ABC components with the value #7399
Conversation
1eb1be3
to
f29bc0d
Compare
The suggestion is fine by me, but I wonder if the extra info won't be confusing to some users. @rubocop-hq/rubocop-core what do you think about this? |
@bbatsov Might be confusing for few, but ideally, without displaying each component the ABC Size value is nothing but just a decimal value IMHO. One would never be sure about what individual components' values are unless count these manually. |
CHANGELOG.md
Outdated
@@ -24,6 +24,7 @@ | |||
* [#7317](https://github.com/rubocop-hq/rubocop/pull/7317): Add new formatter `pacman`. ([@crojasaragonez][]) | |||
* [#6075](https://github.com/rubocop-hq/rubocop/issues/6075): Support `IgnoredPatterns` option for `Naming/MethodName` cop. ([@koic][]) | |||
* [#7335](https://github.com/rubocop-hq/rubocop/pull/7335): Add todo as an alias to disable. `--disable-uncorrectable` will now disable cops using `rubocop:todo` instead of `rubocop:disable`. ([@desheikh][]) | |||
* [#7396](https://github.com/rubocop-hq/rubocop/issues/7396): Display assignments, branches, and, conditions values with the offence. ([@avmnu-sng][]) |
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.
Move this to the master
branch. I'd put it under Changes
there.
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.
Done!
Yeah, that's a fair point. I wonder if we shouldn't put an You'll also have to rebase on top of |
Math.sqrt(@assignment**2 + @branch**2 + @condition**2).round(2) | ||
[ | ||
Math.sqrt(@assignment**2 + @branch**2 + @condition**2).round(2), | ||
"<#{@assignment},#{@branch},#{@condition}>" |
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.
Might be a good idea to add a space after each ,
.
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.
Yep, agreed. Updated.
7bf1cdc
to
b14d52e
Compare
Usually, we write vectors like |
18120d2
to
b066dd0
Compare
CHANGELOG.md
Outdated
@@ -14,6 +14,7 @@ | |||
|
|||
* [#7446](https://github.com/rubocop-hq/rubocop/issues/7446): Add `merge` to list of non-mutating methods. ([@cstyles][]) | |||
* [#7077](https://github.com/rubocop-hq/rubocop/issues/7077): Rename `Unneeded*` cops to `Redundant*` (e.g., `Style/UnneededPercentQ` becomes `Style/RedundantPercentQ`). ([@scottmatthewman][]) | |||
* [#7396](https://github.com/rubocop-hq/rubocop/issues/7396): Display assignments, branches, and, conditions values with the offence. ([@avmnu-sng][]) |
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.
There is an unnecessary comma.
-Display assignments, branches, and, conditions
+Display assignments, branches, and conditions
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.
Fixed!
CHANGELOG.md
Outdated
@@ -4242,3 +4243,4 @@ | |||
[@jfhinchcliffe]: https://github.com/jfhinchcliffe | |||
[@jdkaplan]: https://github.com/jdkaplan | |||
[@cstyles]: https://github.com/cstyles | |||
[@avmnu-sng]: https://github.com/username |
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.
It should be replaced with your account name.
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.
How silly! Fixed
After understanding |
Providing component values gives more insights to fix the offense.
b066dd0
to
7aa807e
Compare
Thanks! |
Providing component values gives more insights to fix the offense.
Metrics/AbcSize: Assignment Branch Condition size for choose_move is too high. [<8, 12, 6> 15.62/15]
definitetly provides more information than
Metrics/AbcSize: Assignment Branch Condition size for choose_move is too high. [15.62/15]
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.