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

Use the RuboCop Rails config #4471

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Conversation

jon-kirwan
Copy link
Contributor

@jon-kirwan jon-kirwan commented Dec 3, 2024

What

Use the RuboCop Rails config and fix the offenses identified by the following cops:

  • Rails/Blank,Rails/Presence,Rails/Present
  • Rails/Delegate
  • Rails/FilePath
  • Rails/Output
  • Rails/RakeEnvironment
  • Rails/TimeZone
  • Style/RedundantParentheses

Why

For consistency with other frontend applications that use the Rails configuration. See #2626

Anything else

  • #2626 mentions how detecting Rails/HelperInstanceVariable offenses would simplify copying code from this repo to another one that enforces such rules. However, it seems that HelperInstanceVariable offenses are still not being detected which is because RuboCop only checks view helpers located in /app/helpers. See https://github.com/rubocop/rubocop-rails/blob/master/config/default.yml#L553-L558
  • Many of the corrections included here seem reasonable to me. The only one I’m uncertain about is Rails/RakeEnvironment. RuboCop autocorrect applies this change by default, but I’m not sure whether we should keep it or if disabling that specific cop might be a better approach

@govuk-ci govuk-ci temporarily deployed to components-gem-pr-4471 December 3, 2024 12:41 Inactive
@jon-kirwan jon-kirwan changed the title Use rubocop rails config Use the RuboCop Rails config Dec 3, 2024
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-4471 December 3, 2024 14:37 Inactive
@jon-kirwan jon-kirwan marked this pull request as ready for review December 4, 2024 13:12
I'm uncertain about this one. According to the documentation: "this cop checks rake task definitions to ensure they include the environment dependency. The environment dependency is essential because it loads the application code for the rake task. Without it, the rake task cannot use application code, like models." However, I don't see this cop being configured in other applications that use the Rails.yml configuration.
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 enforces different standards in the gem than in the frontend apps
2 participants