rubocop-rails
rubocop-rails copied to clipboard
Add new `Rails/ThreeStateBoolean` cop
Closes https://github.com/rubocop/rubocop-rails/issues/337
I had just come to the repo to check if this cop was being worked on already so I could submit it. Thank you for doing this!
@koic Can you merge one of my PR, having StartAfterMigrationVersion
in it, like this PR? So I will create new PRs adding this config option to other existing cops (like AddColumnIndex
, BulkChangeTable
etc) without copying this module over and over.
Could you separate the StartAfterMigrationVersion
config into another PR?
Also, I would like to request the following changes regarding StartAfterMigrationVersion
.
- Rename from
StartAfterMigrationVersion
toMigratedSchemaVersion
- It would be better to set it for the Rails department instead of cop units. Only one value has been migrated. Perhaps user does not want to set it individually for each cop.
I think config/default.yml can be null (~
) by default.
Rails:
# Warnings for db/schema.rb are ignored up to the specified schema version.
# e.g.: MigratedSchemaVersion: '2022_05_05_080819'
MigratedSchemaVersion: ~
User can configure user's .rubocop.yml as follows:
Rails:
MigratedSchemaVersion: '202007130158'
It also supports comparison of the following two timestamp formats:
ActiveRecord::Schema.define(version: 20220505080819) do
and
ActiveRecord::Schema.define(version: 2022_05_05_080819) do
This setting can be applied to all existing cop related for schema migration in the future.
@koic, should I extend Rubocop::Config
with MigratedSchemaVersion
, like here https://github.com/rubocop/rubocop/blob/0d23a0a72fa83baac48f8da930ac8d72cb1bd2e4/lib/rubocop/config.rb#L222-L231? Or create a mixin and add it to some existing cop (for testing)?
TBH, I haven't considered the details of how to implement it yet. So, I can take the configuration task later (maybe a few days later) .
First of all, it is enough if StartAfterMigrationVersion
is dropped from this PR. I will continue to review this PR after that.
Updated with all the suggestions.
@koic Any chance this PR can be reviewed and merged to master? If there is anything I can help you or @fatkodima to finish the PR just let me know. Thanks for creating this cop!
OK. Let's introduce it first.
I think Rails/ThreeStateBooleanColumn
is more specific for the cop name. What do you think?
Thanks!
Thanks @koic & @fatkodima 💯