Manual rubocop fixes #751
Manual rubocop fixes #751
Conversation
| @@ -33,7 +33,7 @@ | |||
|
|
|||
| get "/api/variants?q[product_name_or_sku_cont]=fritos", token: user.spree_api_key | |||
|
|
|||
| skus = JSON.parse(response.body)['variants'].map { |variant| variant['sku'] } | |||
| skus = JSON.parse(response.body)['variants'].map { |x| x['sku'] } | |||
There was a problem hiding this comment.
Why is this beneficial? I'd find variant more descriptive as a variable name than x.
There was a problem hiding this comment.
OK, var shadowing again. I don't mind the x.
There was a problem hiding this comment.
Short local variable names for one liners are ok IMO, but I'll vote forv
|
Aside from the naming nitpick in |
jhawthorn
force-pushed
the
manual_rubocop_fixes
branch
from
4f6b7cd
to
ebdaf49
Compare
|
Updated with more descriptive names in the shadowed variable commit |
| @@ -35,7 +35,7 @@ def check_option_values(option_values) | |||
|
|
|||
| it "can retrieve a list of specific option types" do | |||
| option_type_1 = create(:option_type) | |||
| option_type_2 = create(:option_type) | |||
| create(:option_type) | |||
There was a problem hiding this comment.
If we're here, why not follow "given, when, then" and put this into a before block? option_type1 could even be a let!
jhawthorn
force-pushed
the
manual_rubocop_fixes
branch
from
ebdaf49
to
5cf1e94
Compare
|
rebased |
|
I appreciate some people's comments on things but I'd say just merge it as it is. Its an improvement and none of the suggested changes I read made it significantly better or clearer than the change already happening. Also its a Big Fat Change and the sooner we merge the less hassle it'll be for hawthorn. |
Removes use of inject and unused variables
Remove all assignments flagged by rubocop's Lint/UselessAssignment
This test was previously missing an assertion. Added one to assert the current behaviour
This fixes lines like foo ( 1 + 2 ) + 3 Which is unclear
Fixes Style/NestedParenthesizedCalls
I was on the fence about keeping this cop, but without exception I believe these changes made the code more clear.
This fixes the ruby (and also rubocop) warning about shadowing variables.
jhawthorn
force-pushed
the
manual_rubocop_fixes
branch
from
5cf1e94
to
c831428
Compare
|
Go! |
|
😎 |
|
Thank you for that! |
This fixes most of the remaining rubocop warnings that remained after the autocorrection.
Many of these offenses were not just rubocop warnings, but also ruby syntax warnings. This should mean that all files now pass
ruby -c -w.After this there are only 14 remaining offences, all of which to fix will need some minor behaviour change. To keep reviews simple, all changes made here should not effect behaviour.