-
Notifications
You must be signed in to change notification settings - Fork 4
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
Give ability to execute scripts #14
Conversation
97d0755
to
5a1908d
Compare
(cram | ||
(applies_to example) | ||
(enabled_if | ||
(= true %{env:GITHUB_TESTS=false})) |
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 don't understand this: why enable the tests only on the CI?
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.
We need a container running with Postgres to execute the PostgreSQL test and an SQLite executable available to run the SQLite test. However, opam-repo-ci
doesn't support these kinds of tests... This is the only option I've found to ensure we don't break opam-repo-ci
when releasing new versions.
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! I can't come up with a better alternative either, so let's go with this
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.
We can correct it later if we find something better 👍
Thanks for the review and the suggestions! |
CHANGES: - Various bug fixes (tmattio/omigrate#12, tmattio/omigrate#14, @maiste)
This PR fixes #13 by importing the possibility to execute
SQL script
into theomigrate-sqlite3
driver.This is achieved by directly executing the request instead of trying to prepare it.
It requires #12 to be merged first.