Add new Solidus/SpreeDefaultAddressDeprecated cop #64
Conversation
the-krg
force-pushed
the
the-krg/add-default-address-deprecated-cop
branch
6 times, most recently
from
e7f1f32
to
8af53ee
Compare
| def on_send(node) | ||
| return unless default_address?(node) || default_user_address?(node) | ||
|
|
||
| add_offense(node) |
There was a problem hiding this comment.
Can we add a corrector here to add some autocomplete to this cop as well ?
Seems like we can replace the violation with ship_address or default_user_ship_address in the corrector here as well.
There was a problem hiding this comment.
Thanks for the suggestion! Added!
There was a problem hiding this comment.
Sorry for being late on this. Since this cop just raises a warning, maybe it is better not to use the autocorrection. WDYT? cc @safafa @DanielePalombo
There was a problem hiding this comment.
The PR description mentions that this raises a default offense and also the code looks like it raises an offense rather than a warning.
Hence, raised the question here. @MassimilianoLattanzio
There was a problem hiding this comment.
You're right. It was neither specified in the relative issue. BTW, in our planning, we decided that maybe for too generic cops, it is better to raise a warning instead of an error.
the-krg
force-pushed
the
the-krg/add-default-address-deprecated-cop
branch
from
8af53ee
to
0160bf3
Compare
the-krg
requested review from
piyushswain,
DanielePalombo and
safafa
and removed request for
piyushswain
| add_offense(node) do |corrector| | ||
| corrector.replace(node, node.source.gsub('default_address', 'ship_address')) if default_address?(node) | ||
| corrector.replace(node, node.source.gsub('default_user_address', 'default_user_ship_address')) if default_user_address?(node) |
There was a problem hiding this comment.
I think you should extend AutoCorrector since you're calling corrector.replace here
config/default.yml
Outdated
| Solidus/SpreeDefaultAddressDeprecated: | ||
| Description: 'Checks if user.default_address or user.default_user_address is used and suggest using user.ship_address or user.default_user_ship_address' | ||
| Enabled: true | ||
| VersionAdded: '0.1' |
There was a problem hiding this comment.
Instead of specifying the version here, it's better to have the '<<next>>' string and then run the cut_release rake task as mentioned in the readme. In this way, we can also deploy a new version on GH and Rubygems.
the-krg
force-pushed
the
the-krg/add-default-address-deprecated-cop
branch
from
0160bf3
to
6245e36
Compare
|
Sorry, I'm late on this. Update with the latest suggestions, so we can merge or close it! |
Hey Enzo, thanks for the update. ATM, we paused working on this. As soon as we will restart working on it, I'll review it. |
Description
Add a new cop that checks if user.default_address or user.default_user_address are being used, and warns user to change to user.ship_address or user.default_user_ship_address instead.
Severity:
Wrong Code
Correct Code
Solidus PR Link: solidusio/solidus#3563
Before submitting the PR make sure the following are checked:
main(if not - rebase it)./config/default.yml