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

up_only in migration reports Rails/ReversibleMigration #5821

Closed
morgoth opened this issue Apr 25, 2018 · 1 comment · Fixed by #5828
Closed

up_only in migration reports Rails/ReversibleMigration #5821

morgoth opened this issue Apr 25, 2018 · 1 comment · Fixed by #5828
Labels
enhancement

Comments

@morgoth
Copy link

morgoth commented Apr 25, 2018

Rails 5.2 introduced up_only method http://api.rubyonrails.org/classes/ActiveRecord/Migration.html#method-i-up_only

Rubocop reports Rails/ReversibleMigration violation for:

class DoSomething < ActiveRecord::Migration[5.1]
  def change
    up_only { execute("SET SESSION statement_timeout = 0;") }

    add_column :users, :name, :string
  end
end

Rails/ReversibleMigration: execute is not reversible

rubocop -v
0.55.0
koic added a commit to koic/rubocop that referenced this issue Apr 26, 2018
@koic
[Fix rubocop#5821] Support AR::Migration#up_only for `Rails/Reversi…
3307053 
…bleMigration`

Fixes rubocop#5821.

This PR supports the following `AR::Migration#up_only` method.

```console
% cat db/migrate/example.rb
class AddPublishedToPosts < ActiveRecord::Migration[5.2]
  def change
    add_column :posts, :published, :boolean, default: false

    up_only do
      execute "UPDATE posts SET published = 'true'"
    end
  end
end
```

http://api.rubyonrails.org/classes/ActiveRecord/Migration.html#method-i-up_only

This PR fixes the following false posive when using Rails 5.2.0.

```console
% rubocop db/migrate/example.rb --only Rails/ReversibleMigration
Inspecting 1 file
C

Offenses:

db/migrate/example.rb:6:7: C: Rails/ReversibleMigration: execute is not
reversible.
      execute "UPDATE posts SET published = 'true'"
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected
```
@koic koic mentioned this issue Apr 26, 2018
Merged
8 tasks
@koic
Copy link
Member

koic commented Apr 26, 2018

Thanks for the feedback.
I reproduced it and opend PR #5828.

bbatsov pushed a commit that referenced this issue Apr 26, 2018
@koic @bbatsov
[Fix #5821] Support AR::Migration#up_only for `Rails/ReversibleMigr…
a41dce8 
…ation`

Fixes #5821.

This PR supports the following `AR::Migration#up_only` method.

```console
% cat db/migrate/example.rb
class AddPublishedToPosts < ActiveRecord::Migration[5.2]
  def change
    add_column :posts, :published, :boolean, default: false

    up_only do
      execute "UPDATE posts SET published = 'true'"
    end
  end
end
```

http://api.rubyonrails.org/classes/ActiveRecord/Migration.html#method-i-up_only

This PR fixes the following false posive when using Rails 5.2.0.

```console
% rubocop db/migrate/example.rb --only Rails/ReversibleMigration
Inspecting 1 file
C

Offenses:

db/migrate/example.rb:6:7: C: Rails/ReversibleMigration: execute is not
reversible.
      execute "UPDATE posts SET published = 'true'"
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Fix #5821] Support AR::Migration#up_only for Rails/ReversibleMigration koic/rubocop
3 participants
@morgoth @koic @Drenmi