-
-
Notifications
You must be signed in to change notification settings - Fork 4
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
Add limitation to reimbursement hook #41
Conversation
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.
LFTM
|
||
context 'when target solidus version is lower than the cop minimum solidus version' do | ||
let(:config) { RuboCop::Config.new('AllCops' => { 'TargetSolidusVersion' => '2.10' }) } | ||
|
||
it 'avoid to register an offense when using `#reimbursement_success_hooks.each`' do | ||
expect_no_offenses(<<~RUBY) | ||
reimbursement_success_hooks.each | ||
RUBY | ||
end | ||
|
||
it 'avoid to register an offense when using `#reimbursement_failed_hooks.each`' do | ||
expect_no_offenses(<<~RUBY) | ||
reimbursement_failed_hooks.each | ||
RUBY | ||
end | ||
end |
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 looks to me that these tests are not related with the change.
I would suggest to remove them, minimum_solidus_version 2.11
it's just a configuration and the behavior should be tested on add_offense method.
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.
Tests removed after merging #42
Add minimum Solidus version limitation to the reimbursement hook because the deprecation is introduced starting from Solidus 2.11.
dbda3d0
to
a4c2cee
Compare
Closed in favor of #43 |
Add minimum Solidus version limitation to the reimbursement hook because the deprecation is introduced starting from Solidus 2.11.
Closes #27.