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

New cop rule: IrreversibleMigration #43

Merged

Conversation

AlasdairWallaceMackie
Copy link
Contributor

Should resolve this issue: #30

According to the Sequel documentation, only certain methods should be placed inside a change block in a migration file. Other methods could make the migration irreversible.

This new cop will warn the user under the following conditions inside a change block:

  • They use a method that's not listed in the documentation
  • They use the method #add_primary_key with an array argument instead of a symbol

@@ -17,5 +17,5 @@ Gem::Specification.new do |gem|

gem.required_ruby_version = '>= 2.5'

gem.add_runtime_dependency 'rubocop', '~> 1.0'
gem.add_dependency 'rubocop', '~> 1.0'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Had to make this change due to failed CI build

image

@cyberdelia cyberdelia merged commit 2b10345 into rubocop:master Oct 23, 2024
5 checks passed
@tt
Copy link

tt commented Oct 24, 2024

This introduced an issue where change blocks outside of Sequel migrations are also being detected. See #44 for context.

@soumyaray
Copy link

soumyaray commented Nov 3, 2024

The IrreversibleMigration rule disallows using primary_key and String for create_table in a change block. However, the Sequel documentation example for how to use change in lieu of up/down blocks uses primary_key and String methods for a create_table in a change block. I'm a bit confused here as to why these are not allowed by IrreversibleMigration? Would this new cop also fail the example in the documentation?

Sequel.migration do
  change do
    create_table(:artists) do
      primary_key :id
      String :name, null: false
    end
  end
end

@AlasdairWallaceMackie
Copy link
Contributor Author

@soumyaray Thanks for bringing this to my attention, the cop should be ignoring all methods within a create_table block. I will work on a fix for this

@soumyaray
Copy link

@soumyaray Thanks for bringing this to my attention, the cop should be ignoring all methods within a create_table block. I will work on a fix for this

Thank you so much for your quick handling of this -- really appreciate the work on this gem.

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.

4 participants