-
-
Notifications
You must be signed in to change notification settings - Fork 17
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 a feature to load migrations from a directory #51
Conversation
fa0576a
to
efb4764
Compare
Required PRs were merged, marking TODO as done. Rebased the PR on top of master. It's ready for review. |
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.
I’ve taken a quick look, looks quite promising!
88d026f
to
2d0470a
Compare
This comment was marked as resolved.
This comment was marked as resolved.
17611eb
to
348e92f
Compare
cce7fbe
to
c9597fd
Compare
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.
Thanks for the thorough documentation and testing. I’ve posted quite a few comments, but some are for follow-up work. I think we are getting close to being able to merge!
The subdirectory idea is also much simpler, it could pave the way (in a future PR) to dropping the from_directory dependency and to using our own macro, for the sake of runtime efficiency (the readme should be updated then and tradeoff between compile-time and run-time costs explained)
a1e2be8
to
23d8c19
Compare
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.
lgtm
Thanks for your contribution! |
This PR provides a new feature
from-directory
which allows to specify a directory used to load migrations from and store them in the binary.This implementation assumes that the directory contains
*.sql
files with a documented naming pattern.Closes #37.
TODO:
AsyncMigrations::new
documentation #50, Fix example test #49 and Fix problems and improve tests according tocargo mutants
#47 before we can merge this. Afterwards the PR should be rebased on top of themaster
branch.@cljoly feel free to have a first look and review.