rubocop-rails
rubocop-rails copied to clipboard
Config to ignore old migrations for `Rails/ThreeStateBooleanColumn`
Hello, first-time contributor here 👋
I noticed that the Rails/ThreeStateBooleanColumn
cop reports offenses in old migrations. And as mentioned by @mvz in https://github.com/rubocop/rubocop-rails/discussions/982, fixing these offenses in existing migrations is not practical. Currently, the only way to address this issue is to exclude all offending migrations in the config file, which can add unnecessary noise.
To improve this situation, I propose adding a configuration option for this cop that ignores offenses occurring before a specified migration version. This approach is inspired by the strong_migrations gem. The configuration would look like this:
Rails/ThreeStateBooleanColumn:
StartAfter: 20230415000000 # 0 by default
I'm open to suggestions for a better name for this configuration option. For now, I've used the name from strong_migrations.
Please let me know if you have any feedback or suggestions for improvement. I'm looking forward to your input and would be happy to make any necessary changes.
Thank you!
Before submitting the PR make sure the following are checked:
- [x] The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
- [x] Wrote good commit messages.
- [ ] Commit message starts with
[Fix #issue-number]
(if the related issue exists). - [x] Feature branch is up-to-date with
master
(if not - rebase it). - [x] Squashed related commits together.
- [x] Added tests.
- [x] Ran
bundle exec rake default
. It executes all tests and runs RuboCop on its own code. - [x] Added an entry (file) to the changelog folder named
{change_type}_{change_description}.md
if the new code introduces user-observable changes. See changelog entry format for details. - [ ] If this is a new cop, consider making a corresponding update to the Rails Style Guide.
I tried to introduce such a config option 2 times (https://github.com/rubocop/rubocop-rails/pull/335 and https://github.com/rubocop/rubocop-rails/pull/277), but these are not yet merged to reuse that logic 😄
@fatkodima I'm a little confused by the discussion in those pull requests. Is the intent for offenses in older migrations to never be fixed, not even by using a later migration to change the schema?
These cops are checking migration files when doing their job, so there is no point to do this for older migrations.
It would be nice if schema.rb
file would be checked too (kinda look at violations you already introduced into your app before), but this is better done by tools like https://github.com/gregnavis/active_record_doctor.
Thanks, @fatkodima, that clears things up.
I tried to introduce such a config option 2 times (#335 and #277), but these are not yet merged to reuse that logic 😄
Are there any specific blockers or issues preventing them from being merged?
Since Rails/ThreeStateBooleanColumn
is already in master do you want to open a PR with the StartAfterMigrationVersion
mixin and include it in this rule?
If this PR will be merged first, I will with no problems update with these changes those other two.
IMHO, StartAfter
should apply to all cops that look at migrations. It shouldn't need to be set individually cop-by-cop.
This may be a bit off topic, but in projects where inspecting old migration files seems pointless, I usually disable the inspections as follows:
# .rubocop.yml
inherit_mode:
merge:
- Exclude
AllCops:
Exclude:
- "db/migrate/2020*.rb"
- "db/migrate/2021*.rb"
- "db/migrate/202201*.rb"
- "db/migrate/202202*.rb"
- "db/migrate/202203*.rb"
- "db/migrate/202204*.rb"
- "db/migrate/202205*.rb"
- "db/migrate/202206*.rb"
- "db/migrate/202207*.rb"
- "db/migrate/202208*.rb"
- "db/migrate/202209*.rb"
(In this example, the project here has existed since 2020 and I added this setting on 2022-10-01)
@koic Can you please reconsider this? I am currently in a need for such an option.