-
Notifications
You must be signed in to change notification settings - Fork 631
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
Add annotation hooks for db-specific migration tasks #686
Conversation
ed940b0
to
aed041d
Compare
aed041d
to
37f8b16
Compare
Hi @paul-mannino thanks for this and I apologize for not reviewing this earlier. Since this PR was up I've added an integration test in |
Rails 6 adds a new set of migration tasks for multi-database apps. This change makes the annotate_models_migrate hooks aware of them.
37f8b16
to
5059f91
Compare
@drwl That makes it a bit nicer to test. Lmk if there's anything you want me to change after the latest refactor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, thanks for adding to the integration tests!
@@ -19,91 +82,31 @@ | |||
end | |||
end | |||
|
|||
around(:each) do |example| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neat 👍
Rails 6 adds a new set of migration tasks for multi-database apps. This change makes the annotate_models_migrate hooks aware of them.
Rails 6 adds a new set of migration tasks for multi-database apps. This change makes the annotate_models_migrate hooks aware of them.
This change was a little awkward to test, but I'm happy to change my approach/use a dummy app for more robust integration tests if desired.