Add detailed error message to IrreversibleMigration
#21412
Conversation
| @@ -69,7 +69,7 @@ def record(*command, &block) | |||
| # invert the +command+. | |||
| def inverse_of(command, args, &block) | |||
| method = :"invert_#{command}" | |||
| raise IrreversibleMigration unless respond_to?(method, true) | |||
| raise IrreversibleMigration, "#{method} is not defined, so #{command} is not reversible" unless respond_to?(method, true) | |||
There was a problem hiding this comment.
I don't think we should talk about the method here. It is something internal to Rails, users don't need to care about it. Maybe something like: "This migration uses #{command} that is not automatically reversible, please ensure you have a down method"?
There was a problem hiding this comment.
Oh thank you I will rewrite later !
|
Updated and I add |
| @@ -69,7 +69,7 @@ def record(*command, &block) | |||
| # invert the +command+. | |||
| def inverse_of(command, args, &block) | |||
| method = :"invert_#{command}" | |||
| raise IrreversibleMigration unless respond_to?(method, true) | |||
| raise IrreversibleMigration, "This migration uses #{command} that is not automatically reversible, please define down method" unless respond_to?(method, true) | |||
There was a problem hiding this comment.
If we suggest defining a down method, this means that the user also needs to change def change to def up.
Another option would be a reversible do |dir| block.
There was a problem hiding this comment.
I forgot reversible do |dir| 😢
I will rewrite once more :)
|
It's a good idea to make the exception message more descriptive. At the same time we should improve the rDocs of
|
|
@senny updated! |
|
r? @senny |
| nokogiri (1.6.6.2-x64-mingw32) | ||
| mini_portile (~> 0.6.0) | ||
| nokogiri (1.6.6.2-x86-mingw32) | ||
| mini_portile (~> 0.6.0) |
There was a problem hiding this comment.
I don't think this belongs into the patch.
There was a problem hiding this comment.
I will remove now this
…_error_msg Add detailed error message to `IrreversibleMigration`
|
@yui-knk thanks 💛 I made some minor modifications in the merge commit. Sadly I forgot to check wether the commits are squashed 😓, please amend review changes directly to keep the PR in a mergable state at all times. This helps reviewers a lot. |
|
@yui-knk could you point me to the place where you found that sentence? |
|
@senny I found nothing related with "Do not squash" in Rails Guide. |
|
@senny @yui-knk It makes sense for big PRs but I don't think it's practical with small ones like this.
|
No description provided.