-
-
Notifications
You must be signed in to change notification settings - Fork 277
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 RSpec/IndexedLet cop #1615
Add RSpec/IndexedLet cop #1615
Conversation
4fcee85
to
39a0d09
Compare
Not sure why that check fails, works fine locally |
I see |
Not necessarily, |
e70499b
to
eb817da
Compare
eb817da
to
03f0a0e
Compare
@pirj Addressed everything, ready for another round 🙂 |
let(:max_repeats) { 1 } | ||
let(:cop_config) { { 'MaxRepeats' => max_repeats } } | ||
|
||
specify do |
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 sometimes outputs like following, so it seems better to pass a description to it
or specify
like other specs.
RuboCop::Cop::RSpec::IndexedLet
is expected to eq describe SomeService do
let(:item_1) { create(:item) }
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This `let` ...ame. Please give it a meaningful names, use create_list or move creation to the `before` block.
end
I think it would be good to have the following as test cases as well. describe SomeService do
let(:item_1) { create(:item) }
let(:foo_item_1) { create(:item) }
end
describe SomeService do
let(:item_1) { create(:item) }
let("item_2") { create(:item) }
end |
Co-authored-by: Benjamin Quorning <22333+bquorning@users.noreply.github.com>
Co-authored-by: Benjamin Quorning <22333+bquorning@users.noreply.github.com>
Co-authored-by: Benjamin Quorning <22333+bquorning@users.noreply.github.com>
Co-authored-by: Benjamin Quorning <22333+bquorning@users.noreply.github.com>
Applied all suggesstions, updated specs and docs, thanks 🙂 |
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.
Thank you for doing a great work ❤️
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.
Thank you!
A few nitpicks below.
Updated 🙂 |
Thank you, @DmitryTsepelev ! |
Oops, it looks like we forgot to squash commits before merging 😅 |
Adds the cop that tries to find indexed let statements (
item_1
,item_2
, ...):Before submitting the PR make sure the following are checked:
master
(if not - rebase it).CHANGELOG.md
if the new code introduces user-observable changes.bundle exec rake
) passes (be sure to run this locally, since it may produce updated documentation that you will need to commit).If you have created a new cop:
config/default.yml
.Enabled: pending
inconfig/default.yml
.Enabled: true
in.rubocop.yml
.VersionAdded: "<<next>>"
indefault/config.yml
.If you have modified an existing cop's configuration options:
VersionChanged: "<<next>>"
inconfig/default.yml
.