Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Apply required migrations #4

Merged
merged 1 commit into from
Oct 11, 2022
Merged

Conversation

maiste
Copy link
Collaborator

@maiste maiste commented Oct 11, 2022

This PR updates the code only to execute the migrations that are not already applied. It provides a new flag force to apply all the migrations to ensure we can keep the old behaviour.

The comparison is made this way:

Compare result Bound (aka force case)
Up $version > origin \Leftrightarrow version - origin > 0$ 0 : $\forall version \in \mathbb{N}^{+}, version - 0 > 0 \Leftrightarrow True $
Down $version <= origin \Leftrightarrow version - origin <= 0$ maxint : $\forall version \in \mathbb{N}^{+}, version - maxint <= 0 \Leftrightarrow True $

Warning
This introduces a breaking change in the API. Maybe the release will need an upgrade in the Minor version.

It requires #3 to be merged before merging.

@tmattio
Copy link
Owner

tmattio commented Oct 11, 2022

Thanks @maiste! I just merged #3. Could you rebase on top of master?

@maiste maiste force-pushed the partial-migrations branch from a1c265b to fdb90c3 Compare October 11, 2022 15:37
@maiste
Copy link
Collaborator Author

maiste commented Oct 11, 2022

Done ✔️

@tmattio
Copy link
Owner

tmattio commented Oct 11, 2022

Thanks! Do you think we could add a cram test for this? I'm happy to merge if you won't have time in the coming days so this doesn't block you on ocurrent

@maiste
Copy link
Collaborator Author

maiste commented Oct 11, 2022

Thanks! We could merge it like this if that's ok with you?
I will add the tests during the week in a separated PR as I feel we need to have a complete set of tests for the entire systemes. What do you think? 😄

@tmattio
Copy link
Owner

tmattio commented Oct 11, 2022

Sure, let's merge now!

@tmattio tmattio merged commit 367156a into tmattio:master Oct 11, 2022
tmattio added a commit to tmattio/opam-repository that referenced this pull request Oct 11, 2022
CHANGES:

- Support partial migrations (tmattio/omigrate#4, @maiste)
- New Sqlite3 driver (tmattio/omigrate#3, @maiste)
- Support optional auth with psql driver (tmattio/omigrate#2, @maiste)
@maiste maiste deleted the partial-migrations branch October 12, 2022 08:00
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants