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

Separate Ruby (rubocop-github) from Rails (rubocop-github-rails) cops and rules to disambiguate rule inheritance from custom cops #121

Merged
merged 1 commit into from
Oct 13, 2022

Conversation

bensheldon
Copy link
Contributor

@bensheldon bensheldon commented Oct 12, 2022

This creates clearer distinctions between

  • opinionated rule configuration inheritance e.g. inherit_from, and
  • custom cops and their default configuration, which can be separately require:'d

This also creates stronger separation between the generic-Ruby rules (e.g. config/default.yml) and the Rails-specific rules (e.g. config/rails.yml). I believe that will become necessary because of #119, because the current implementation requires all the cops (Ruby and Rails) which means that they will all become Enabled/Pending (or whatever the *ByDefault is) even if the developer only wants the generic-Ruby rules. This PR makes it so that default.yml will only load/require the generic-Ruby cops and not all the Rails cops, and vice versa.

I followed the pattern used in rubocop-rails and rubocop-performance to inject! Rubocop configuration/rules, along with their pattern of having Enabled: pending status.

This should be fully backward compatible too and not require any changes by consumers.

Also, I learned a lot about how to package a Rubocop gem and plan to go suggest these changes too on rubocop-rails-accessibility (though slightly differently because that gem is just cops).

Copy link
Contributor

@elenatanasoiu elenatanasoiu left a comment

Choose a reason for hiding this comment

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

Thanks for working with us on this and paving the way for #119! ❤️

Copy link
Member

@issyl0 issyl0 left a comment

Choose a reason for hiding this comment

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

This has to be merged ahead of #119 right?

@sampart
Copy link
Contributor

sampart commented Oct 13, 2022

Where does config/rails_cops.yml get picked up? I can't see any references to it.

@sampart
Copy link
Contributor

sampart commented Oct 13, 2022

Never mind, I think I understand this now - it's the config stuff further down in the PR.

Copy link
Contributor

@sampart sampart left a comment

Choose a reason for hiding this comment

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

I won't claim to have completely wrapped my head around this, but looks fine to me.

…cops and rules to disambiguate rule inheritance from custom cops
@bensheldon bensheldon merged commit bbe922b into master Oct 13, 2022
@bensheldon bensheldon deleted the separate_rules_from_cops branch October 13, 2022 15:05
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