makandra / makandra-rubocop

makandra's default Rubocop configuration
MIT License
6 stars 1 forks source link

Adding default configs for rubocop-factory_bot, rubocop-rake and rubocop-capybara #45

Closed denzelem closed 5 months ago

denzelem commented 5 months ago

The new rubocop version gives you hints about recommended rubocops. Does it makes sense to add makandra defaults for the following gems just like we've done it for rubocop-rails and rubocop-rspec?

FLeinzi commented 5 months ago

The suggestions are not a new feature, but most of our projects afaik have this as default:

AllCops:
  SuggestExtensions: false

I haven't looked into rubocop-rake yet, but the other two have the problem that they share cops with rubocop-rspec. That means, that e.g. FactoryBot/ConsistentParenthesesStyle and RSpec/FactoryBot/ConsistentParenthesesStyle are defined. I think that's the reason why @foobear hasn't activated them yet. One has to take special care about that when upgrading.

But on the other side I would agree to your request because this would make our code more consistent. So :+1: to more defaults.

foobear commented 5 months ago

I'm not sure how many projects use these suggested rubocop sets, but I believe a few at best.

I would encourage everyone to try and integrate them with their projects, if possible. Having a set of default rules probably makes sense, but deciding on a custom ruleset might be difficult without having seen those cops act in real-world projects.

FLeinzi commented 5 months ago

As I said, some of them are already defined in rspec.yml. They already have decisions in our current version of makandra-rubocop.

Don't know which is the best way to handle those. You could either remove them from rspec.yml or just disable them there to not have to manage them at two different places.

In the rubocop-rspec repo they are configured in config/obsoletion.yml.

denzelem commented 5 months ago

@FLeinzi Thanks for this edge case of possible overlapping cops. For most projects it should be good enough if the default is consistent between the overlapping cops. In case a project deviates from the default config, it needs to take care of this on its own.

@foobear I wasn't that clear when opening that issue. I totally agree with you and would start extracting some useful default configuration after using the cops in some large projects. The issue was more like a "do we want to have a default here" to take further steps.

foobear commented 5 months ago

That's how I understood the issue. :slightly_smiling_face: I'm just a bit on the edge about deciding on defaults now. But providing defaults probably helps with those cops being adopted in more projects, so I'm also fine with taking further steps. :+1: