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.

Sign up for GitHub

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

Jump to bottom

AllCops: MigratedSchemaVersion ignored by many cops #1410

Closed
krcm0209 opened this issue Jan 13, 2025 · 0 comments · Fixed by #1411
Closed

AllCops: MigratedSchemaVersion ignored by many cops #1410

krcm0209 opened this issue Jan 13, 2025 · 0 comments · Fixed by #1411

Comments

@krcm0209
Copy link

krcm0209 commented Jan 13, 2025 •
edited
Loading

Expected behavior

Adding

AllCops:
  MigratedSchemaVersion: '20250106221634'

ignores all migration files on or before the given date-time, for all cops.

Actual behavior

Many cops still report violations, such as those from

  • Layout/*
  • Lint/*
  • Naming/*
  • Rails/*
  • Style/*

RuboCop version

$ [bundle exec] rubocop -V
1.70.0 (using Parser 3.3.6.0, rubocop-ast 1.37.0, analyzing as Ruby 3.4, running on ruby 3.4.1) [aarch64-linux]
  - rubocop-minitest 0.36.0
  - rubocop-performance 1.23.1
  - rubocop-rails 2.28.0

Related: #1383 (comment)
koic added a commit to koic/rubocop-rails that referenced this issue Jan 14, 2025
@koic
[Fix rubocop#1410] Make registered cops aware of `AllCops: MigratedSc…
11321b8 
…hemaVersion`

When implementing rubocop#1383, the detection of `AllCops: MigratedSchemaVersion` was
initially considered only for cops related to migrations.

However, feedback received later, such as in rubocop#1410, indicated that it is also expected to apply to
`Style`, `Lint`, and other categories. This suggestion is reasonable, as warnings for migrated files
may not be limited to database columns but could also include Ruby programming logic.
Excluding `Style` and `Lint` from this consideration would not align with this feedback.

This PR modifies the behavior so that all registered cops can detect the value of `AllCops: MigratedSchemaVersion`.

Fixes rubocop#1410.
koic added a commit to koic/rubocop-rails that referenced this issue Jan 14, 2025
@koic
fixup! [Fix rubocop#1410] Make registered cops aware of `AllCops: Mig…
2caf790 
…ratedSchemaVersion`
koic added a commit to koic/rubocop-rails that referenced this issue Jan 14, 2025
@koic
[Fix rubocop#1410] Make registered cops aware of `AllCops: MigratedSc…
cc7b502 
…hemaVersion`

When implementing rubocop#1383, the detection of `AllCops: MigratedSchemaVersion` was
initially considered only for cops related to migrations.

However, feedback received later, such as in rubocop#1410, indicated that it is also expected to apply to
`Style`, `Lint`, and other categories. This suggestion is reasonable, as warnings for migrated files
may not be limited to database columns but could also include Ruby programming logic.
Excluding `Style` and `Lint` from this consideration would not align with this feedback.

This PR modifies the behavior so that all registered cops can detect the value of `AllCops: MigratedSchemaVersion`.

Fixes rubocop#1410.
koic added a commit to koic/rubocop-rails that referenced this issue Jan 14, 2025
@koic
fixup! [Fix rubocop#1410] Make registered cops aware of `AllCops: Mig…
aeaf11f 
…ratedSchemaVersion`
@koic koic mentioned this issue Jan 14, 2025
Merged
9 tasks
koic added a commit to koic/rubocop-rails that referenced this issue Jan 14, 2025
@koic
[Fix rubocop#1410] Make registered cops aware of `AllCops: MigratedSc…
8a06cfd 
…hemaVersion`

When implementing rubocop#1383, the detection of `AllCops: MigratedSchemaVersion` was
initially considered only for cops related to migrations.

However, feedback received later, such as in rubocop#1410, indicated that it is also expected to apply to
`Style`, `Lint`, and other categories. This suggestion is reasonable, as warnings for migrated files
may not be limited to database columns but could also include Ruby programming logic.
Excluding `Style` and `Lint` from this consideration would not align with this feedback.

This PR modifies the behavior so that all registered cops can detect the value of `AllCops: MigratedSchemaVersion`.

Fixes rubocop#1410.
@koic koic closed this as completed in 4214608 Jan 18, 2025
koic added a commit that referenced this issue Jan 18, 2025
@koic
Merge pull request #1411 from koic/make_registered_cops_aware_of_all_…
ffdcd44 
…cops_migrated_schema_version

[Fix #1410] Make registered cops aware of `AllCops: MigratedSchemaVersion`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Fix #1410] Make registered cops aware of AllCops: MigratedSchemaVersion koic/rubocop-rails
1 participant
@krcm0209