Skip to content
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

Disable MultipleMemoizedHelpers Cop #270

Merged
merged 1 commit into from
Jun 10, 2021
Merged

Conversation

sihu
Copy link
Member

@sihu sihu commented Jun 10, 2021

rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/MultipleMemoizedHelpers

The cop would check if example groups contain too many let and subject calls. As discussed with @coorasse this does not lead to better design in our opinion. People would change the amount of allowed let-statements or trick and use instance variables in before, if they need more. so i’d rather like to disable it.

@sihu sihu requested review from schmijos, coorasse and macav June 10, 2021 07:59
@sihu sihu self-assigned this Jun 10, 2021
@coorasse
Copy link
Member

There seems to be a lot of discussions behind this topic. I am sure it was a good idea to disable it on one11 and possible on other projects where we have already multiple warnings due to this cop.
I proposed it to Simon to open a PR for the ASG but I didn't read the discussions entirely, so I don't have the feeling to be enough prepared on this topic.
I'd just do it because I am used to use many lets and I don't want to change the way I write tests, but maybe there are interesting elements in the discussions that lead to this cop?

I'll take the time next weekend to read about the topic an give my opinion in a more "informed" way.
In the meantime, I have no problems in disabling it for existing projects because it might lead to too many hours of refactorings needed.

Copy link
Member

@schmijos schmijos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GTM!

Ceterum censeo standardrb esse introducam

@sihu sihu assigned coorasse and unassigned sihu Jun 10, 2021
@coorasse coorasse merged commit 3fbdfba into master Jun 10, 2021
@coorasse coorasse deleted the feature/disable-let-cop branch June 10, 2021 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants