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

[SDQ-2009] Prevent disabling rubocop, except for existing exceptions in the monolith #18

Conversation

rhunterharris
Copy link
Contributor

Per #17 we want to enforce disabling Lint/MissingSuper, except for certain parent classes. As such, we've added this directive. Following up on #14.

We have whitelisted all rubocop disable types present in the monolith here. This conveniently provides an in-code documentation of directives we need to follow up on and disable or change the rules for.

All monolith rubocop:disable categories are allowlisted here, excluding:
Lint/MissingSuper (https://github.com/sendoso/rubocop-sendoso/pull/17/files)
Rails/SkipsModelValidations (#15)

@rhunterharris rhunterharris marked this pull request as ready for review July 13, 2023 15:06
Style/DisableCopsWithinSourceCodeDirective:
Description: Detects comments to enable/disable RuboCop. This is useful if want to make sure that every RuboCop error gets fixed and not quickly disabled with a comment.
Enabled: true
AllowedCops:
Copy link
Contributor

Choose a reason for hiding this comment

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

so we are allowing the following cops to be disabled?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that is correct.

Ideally we'll go through this list, relax/tighten rules as needed, then remove from this list.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks good. Wondering if this list can be moved to https://github.com/sendoso/sendoso/blob/master/.rubocop.yml as this seems specific to that repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is located here under the assumption that other repos disable the same cops that the monolith, thus keeping this as a central source of truth for rubocop standards.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Makes sense 👍🏾

Copy link
Collaborator

@gauravs gauravs left a comment

Choose a reason for hiding this comment

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

Looks good!

@rhunterharris rhunterharris merged commit b0e1b2c into master Jul 13, 2023
4 checks passed
@rhunterharris rhunterharris deleted the SDQ-2009-disable-disabling-rubocop-to-enforce-whitelisted-classes-for-missing-super branch July 13, 2023 19:22
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.

Rubocop is frequently being disabled in our code - start to disallow disabling Rspec
3 participants