-
-
Notifications
You must be signed in to change notification settings - Fork 282
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 instruction for running evolutions with Slick using compile-time DI #402
base: main
Are you sure you want to change the base?
Add instruction for running evolutions with Slick using compile-time DI #402
Conversation
Codecov Report
@@ Coverage Diff @@
## master #402 +/- ##
======================================
Coverage 7.71% 7.71%
======================================
Files 19 19
Lines 842 842
Branches 9 8 -1
======================================
Hits 65 65
Misses 777 777 Continue to review full report at Codecov.
|
@@ -41,6 +41,9 @@ Note there is no need to add the Play `evolutions` component to your dependencie | |||
|
|||
> **Note**: You can see th | |||
|
|||
#### Running evolutions with Slick using compile-time DI | |||
If you are using Slick, you will also need to mix in the `SlickEvolutionsComponents` trait in addition to the `EvolutionsComponents` trait to `ApplicationComponents`. Else evolutions will not recognize the Slick database configurations in the configuration files. | |||
|
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.
It could be good to add a code sample here. You can see the recommended way of adding code samples here:
https://playframework.com/documentation/2.5.x/Documentation#Code-samples
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.
Actually, what do you think about changing the name to SlickDBComponents
instead of SlickEvolutionsComponents
? That would make things more straightforward since this basically replaces DBComponents
. Then we can deprecate the whole trait instead of just that api
method.
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.
@gmethvin +1.
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.
Thank you, @annikasirapandji.
Just a small comment to add a code sample, but besides that, this looks good to me.
Hi @annikasirapandji, could you please rebase against master branch? Also, if you still have time, could you take a look at the comments made above? If not, please let me know and I can do it later. :-) Moreover, thanks for contributing and sorry for the huge delay to move this forward. |
How can I help to get this really helpful pull request merged? Should I add a code snippet? |
No description provided.